Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use exec collectors instead of http for support bundles #186

Merged

Conversation

FourSigma
Copy link
Contributor

@FourSigma FourSigma commented Jun 6, 2024

What this PR does / why we need it:

  • Remove http collectors (they were not working)
  • Uses exec collectors via curl
  • Updates /license/info and /app/info SDK endpoints to return additional details

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Steps to reproduce

Does this PR introduce a user-facing change?

- Returns additional license-info and app-info details on respective SDK HTTP endpoints.  
- Support bundle spec updated to extract license, app, history, and release information via an exec collector.  

Notice the new replicated-app* exec folders
Screenshot 2024-06-11 at 3 48 56 PM

Does this PR require documentation?

@FourSigma FourSigma force-pushed the siva/sc-99532/collect-instance-and-bundle-details-in-support branch from 56e0f53 to ebffbc7 Compare June 10, 2024 15:17
pkg/handlers/app.go Outdated Show resolved Hide resolved
@@ -18,6 +18,50 @@ stringData:
name: {{ include "replicated.supportBundleName" . }}
spec:
collectors:
- exec:
name: replicated-app-updates
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought - could we have the name be common between these so that the files end up in the same directory in the support bundle? any reason we might not want that?

looking at these docs: https://troubleshoot.sh/docs/collect/exec/#included-resources

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is really good idea Craig!

@cbodonnell
Copy link
Contributor

@FourSigma could we add something small to these tests to validate that we get these files in the support bundle?

for helm install:

- name: Validate support bundle contents
run: |
./support-bundle --load-cluster-specs --interactive=false
tar xzf support-bundle-*.tar.gz
if ! ls support-bundle-*/secrets/*/replicated-instance-report/report.json; then
echo "Did not find replicated-instance-report in support bundle"
exit 1
fi
if ! ls support-bundle-*/secrets/*/replicated-custom-app-metrics-report/report.json; then
echo "Did not find replicated-custom-app-metrics-report in support bundle"
exit 1
fi
if ! ls support-bundle-*/replicated/logs/*/*.log; then
echo "Did not find replicated pod logs in support bundle"
exit 1
fi
rm -rf support-bundle-*

for helm template | kubectl apply:

- name: Validate support bundle contents
run: |
./support-bundle --load-cluster-specs --interactive=false
tar xzf support-bundle-*.tar.gz
if ! ls support-bundle-*/secrets/*/replicated-instance-report/report.json; then
echo "Did not find replicated-instance-report in support bundle"
exit 1
fi
if ! ls support-bundle-*/secrets/*/replicated-custom-app-metrics-report/report.json; then
echo "Did not find replicated-custom-app-metrics-report in support bundle"
exit 1
fi
if ! ls support-bundle-*/replicated/logs/*/*.log; then
echo "Did not find replicated pod logs in support bundle"
exit 1
fi
rm -rf support-bundle-*

@FourSigma
Copy link
Contributor Author

@FourSigma could we add something small to these tests to validate that we get these files in the support bundle?

for helm install:

- name: Validate support bundle contents
run: |
./support-bundle --load-cluster-specs --interactive=false
tar xzf support-bundle-*.tar.gz
if ! ls support-bundle-*/secrets/*/replicated-instance-report/report.json; then
echo "Did not find replicated-instance-report in support bundle"
exit 1
fi
if ! ls support-bundle-*/secrets/*/replicated-custom-app-metrics-report/report.json; then
echo "Did not find replicated-custom-app-metrics-report in support bundle"
exit 1
fi
if ! ls support-bundle-*/replicated/logs/*/*.log; then
echo "Did not find replicated pod logs in support bundle"
exit 1
fi
rm -rf support-bundle-*

for helm template | kubectl apply:

- name: Validate support bundle contents
run: |
./support-bundle --load-cluster-specs --interactive=false
tar xzf support-bundle-*.tar.gz
if ! ls support-bundle-*/secrets/*/replicated-instance-report/report.json; then
echo "Did not find replicated-instance-report in support bundle"
exit 1
fi
if ! ls support-bundle-*/secrets/*/replicated-custom-app-metrics-report/report.json; then
echo "Did not find replicated-custom-app-metrics-report in support bundle"
exit 1
fi
if ! ls support-bundle-*/replicated/logs/*/*.log; then
echo "Did not find replicated pod logs in support bundle"
exit 1
fi
rm -rf support-bundle-*

@cbodonnell Will do. My only concern is if the Dockerfile that we use for GH Action testing has curl in it.

cbodonnell
cbodonnell previously approved these changes Jun 12, 2024
@cbodonnell
Copy link
Contributor

cbodonnell commented Jun 12, 2024

@cbodonnell Will do. My only concern is if the Dockerfile that we use for GH Action testing has curl in it.

@FourSigma I believe the tests use the Chainguard build process as well (source). That Dockerfile seems to only be there for local testing at this point. It might actually be good to add curl to that as well if it's fairly easy to do. Though I'll leave that up to you.

I'll approve the PR as it all looks good to me, but ping here again if you do end up adding that test snippet or anything else.

@FourSigma FourSigma force-pushed the siva/sc-99532/collect-instance-and-bundle-details-in-support branch from cc46065 to 2343ca3 Compare June 12, 2024 17:35
@FourSigma FourSigma merged commit b4ed1b1 into main Jun 12, 2024
11 checks passed
@FourSigma FourSigma deleted the siva/sc-99532/collect-instance-and-bundle-details-in-support branch June 12, 2024 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants