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

Remove bcc image #2094

Merged
merged 4 commits into from
Oct 4, 2023
Merged

Remove bcc image #2094

merged 4 commits into from
Oct 4, 2023

Conversation

eiffel-fl
Copy link
Member

Hi.

This PR deprecates the bcc image and remove any mention of default as we only have one image now.

Best regards.

Copy link
Member

@mauriciovasquezbernal mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

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

Looks good mostly, still missing some places to be updated:

pkg/resources/manifests/deploy.yaml Outdated Show resolved Hide resolved
docs/install.md Outdated Show resolved Hide resolved
.github/workflows/inspektor-gadget.yml Outdated Show resolved Hide resolved
gadget-container/entrypoint.sh Show resolved Hide resolved
pkg/gadget-collection/gadgets/profile/cpu/gadget.go Outdated Show resolved Hide resolved
charts/gadget/templates/daemonset.yaml Outdated Show resolved Hide resolved
Dockerfiles/gadget.Dockerfile Outdated Show resolved Hide resolved
This reverts commit dd8ddc0.

Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
This reverts commit adf4c78.

Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
@eiffel-fl eiffel-fl force-pushed the francis/deprecate-bcc branch 7 times, most recently from 1cd9dcb to 8e6334e Compare October 3, 2023 11:51
Copy link
Member

@mauriciovasquezbernal mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

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

LGTM from code inspection. Just one nit, the title of the PR and some commits should be updated since it's not deprecating but removing BCC support.

@eiffel-fl eiffel-fl changed the title Deprecate bcc image Remove bcc image Oct 4, 2023
We now only have one flavour of the gadget container image, there is no sense
naming it default.

The removal of standardtracer code was done by the following gopatch [1]:
@@
@@
	t.tracer, err = tracer.NewTracer(...)
	if err != nil {
-		trace.Status.OperationWarning = fmt.Sprint(...)

-		// fallback to standard tracer
-		log.Infof(...)

-		t.tracer, err = standardtracer.NewTracer(...)
-		if err != nil {
-			trace.Status.OperationError = fmt.Sprintf("failed to create tracer: %s", err)
-			return
-		}
+		trace.Status.OperationError = fmt.Sprintf("failed to create tracer: %s", err)
+		return
	}

Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
[1]: https://github.com/uber-go/gopatch
Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
@eiffel-fl
Copy link
Member Author

Thank you for the review!

@eiffel-fl eiffel-fl merged commit 01ea167 into main Oct 4, 2023
28 checks passed
@eiffel-fl eiffel-fl deleted the francis/deprecate-bcc branch October 4, 2023 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove BCC support [EPIC]: Move BCC tools to a legacy mode
2 participants