Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

pkg/components: add Inspektor Gadget component #1076

Merged
merged 5 commits into from
Oct 22, 2020
Merged

pkg/components: add Inspektor Gadget component #1076

merged 5 commits into from
Oct 22, 2020

Conversation

iaguis
Copy link
Contributor

@iaguis iaguis commented Oct 14, 2020

Fixes #1063

@iaguis iaguis force-pushed the iaguis/inspektor-gadget branch 4 times, most recently from 9dcad0b to ff3fc5b Compare October 14, 2020 12:58
invidian
invidian previously approved these changes Oct 15, 2020
View reviewed changes
Copy link
Member

@invidian invidian left a comment

Choose a reason for hiding this comment

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

Just some nits, otherwise LGTM, but I'm curious why CI is failing.

assets/charts/components/inspektor-gadget/templates/psp.yaml Outdated Show resolved Hide resolved
assets/charts/components/inspektor-gadget/values.yaml Outdated Show resolved Hide resolved
@iaguis iaguis force-pushed the iaguis/inspektor-gadget branch 4 times, most recently from 94eef77 to 150bcc1 Compare October 15, 2020 15:14
@iaguis
Copy link
Contributor Author

iaguis commented Oct 16, 2020

I've changed the namespace to gadget, for some reasoning see inspektor-gadget/inspektor-gadget#140 (comment).

I'm curious why CI is failing.

I was using WaitForDeployment() instead of ẀaitForDaemonSet() 🤦

@iaguis iaguis force-pushed the iaguis/inspektor-gadget branch from 150bcc1 to 6dac435 Compare October 16, 2020 10:44
@iaguis
Copy link
Contributor Author

iaguis commented Oct 16, 2020

I've changed the namespace to gadget, for some reasoning see kinvolk/inspektor-gadget#140 (comment).

After thinking about it some more and discussing with others I think what makes most sense is to keep IG in the kube-system namespace so it works out of the box with the current kubectl gadget plugin. Since it will have its own PSP this should be OK. If/when upstream IG changes its namespace to gadget we can change it too.

I changed the PR accordingly.

@invidian
Copy link
Member

Odd that CI didn't get triggered.

invidian
invidian previously requested changes Oct 19, 2020
View reviewed changes
Copy link
Member

@invidian invidian left a comment

Choose a reason for hiding this comment

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

Just one broken thing, otherwise LGTM.

docs/configuration-reference/components/inspektor-gadget.md Outdated Show resolved Hide resolved
@iaguis iaguis force-pushed the iaguis/inspektor-gadget branch from 6dac435 to 8129b0b Compare October 19, 2020 16:44
knrt10
knrt10 suggested changes Oct 22, 2020
View reviewed changes
Copy link
Member

@knrt10 knrt10 left a comment

Choose a reason for hiding this comment

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

One small NIT, rest LGTM

@iaguis iaguis force-pushed the iaguis/inspektor-gadget branch from 8129b0b to 10705c9 Compare October 22, 2020 11:09
@iaguis iaguis requested review from knrt10 and invidian October 22, 2020 11:12
knrt10
knrt10 approved these changes Oct 22, 2020
View reviewed changes
Copy link
Member

@knrt10 knrt10 left a comment

Choose a reason for hiding this comment

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

Looks great. +1

@knrt10 knrt10 dismissed invidian’s stale review October 22, 2020 11:54

change addressed

@iaguis
Copy link
Contributor Author

iaguis commented Oct 22, 2020

I'll take @invidian's lgtm and @knrt10's one and merge this.

@iaguis iaguis merged commit 3c901c0 into master Oct 22, 2020
@iaguis iaguis deleted the iaguis/inspektor-gadget branch October 22, 2020 13:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@knrt10 knrt10 knrt10 approved these changes

@invidian invidian Awaiting requested review from invidian

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add Inspektor Gadget component
3 participants
@iaguis @invidian @knrt10