Allow IG to deploy into a custom namespace #2250

burak-ok · 2023-11-27T13:10:01Z

Allow IG to deploy into a custom namespace

This PR implements the deployment of Inspektor Gadget into a custom Kubernetes Namespace

mauriciovasquezbernal

I'm wondering about the UX of this, specially regarding the --gadget-namespace parameter.

I did the following:

$ ./kubectl-gadget deploy --experimental --gadget-namespace foo
....

$ ./kubectl-gadget trace exec 
INFO[0000] Experimental features enabled                
K8S.NODE               K8S.NAMESPACE          K8S.POD                K8S.CONTAINER          PID        PPID       COMM        RET ARGS                        
Error: running gadget: getting target nodes: get gadget pods: no gadget pods found. Is Inspektor Gadget deployed?

I know it can be fixed by passing --gadget-parameter, but do we really want the user having to specify this parameter each time?

Maybe it's time to extend / implement a config concept, so things like namespace are stored there and the user doesn't have to set it each time. I suppose it'll be useful in the future to store other things like credentials, user preferences, etc.

What do you folks think @alban @flyth?

pkg/runtime/grpc/grpc-runtime.go

cmd/kubectl-gadget/main.go

eiffel-fl

I have a déjà-vu regarding the code.
I will test it but can you please rebase it?

eiffel-fl

I tested it and I have a problem with traceloop:

$ ./kubectl-gadget deploy --gadget-namespace foobar --image-pull-policy Never
Creating Namespace/foobar...
Creating ServiceAccount/gadget...
Creating ClusterRole/gadget-cluster-role...
Creating ClusterRoleBinding/gadget-cluster-role-binding...
Creating DaemonSet/gadget...
Creating CustomResourceDefinition/traces.gadget.kinvolk.io...
Waiting for gadget pod(s) to be ready...
0/1 gadget pod(s) ready
1/1 gadget pod(s) ready
Retrieving Gadget Catalog...
Inspektor Gadget successfully deployed
$ ../kubectl get pod -n foobar
NAME           READY   STATUS    RESTARTS   AGE
gadget-228df   1/1     Running   0          6s
$ ./kubectl-gadget traceloop start
Error: running gadget: no gadget pods found. Is Inspektor Gadget deployed?

I guess this is related to having gadget being hardcoded below:

EDIT: Same with trace exec:

$ ./kubectl-gadget trace exec
K8S.NODE            K8S.NAMESPACE       K8S.POD             K8S.CONTAINER      PID       PPID      COMM      RET ARGS                    
Error: running gadget: getting target nodes: get gadget pods: no gadget pods found. Is Inspektor Gadget deployed?

It seems we need to give the namespace itself:

$ ./kubectl-gadget trace exec --gadget-namespace foobar -A
K8S.NODE            K8S.NAMESPACE       K8S.POD             K8S.CONTAINER      PID       PPID      COMM      RET ARGS                    
minikube-docker     foobar              gadget-228df        gadget             62734     62725     gadgettr… 0   /bin/gadgettracermanage…

Can we avoid the above in case there is only "gadget" namespace? So, we can search for "gadget" pod, getting the namespace and if there is only one, we are set.

cmd/kubectl-gadget/utils/flags.go

cmd/kubectl-gadget/utils/trace.go

pkg/runtime/grpc/grpc-runtime.go

cmd/kubectl-gadget/main.go

burak-ok · 2023-11-29T17:03:50Z

Update: traceloop and advise should work now with custom namespaces

Commit 0859d33 is there to decouple the utils module from grpcruntime.
Hopefully this is only temporary, since traceloop and advise sometime will be "migrated to the registry"
Before that commit the grpcruntime module had a dependency on utils so we couldn't import anything from grpcruntime from there (cyclic dependency)

eiffel-fl

I tested it again and traceloop works fine:

$ ./kubectl-gadget traceloop show 0f1a8dfb52f71ce --gadget-namespace foobar | head
INFO[0000] Experimental features enabled                
CPU PID              COMM             SYSCALL            PARAMS                                   RET
6   22985            coredns          futex                                                       -1…
6   22985            coredns          nanosleep          rqtp=0xc000085f10, rmtp=0x0              0

I am nonetheless still wondering if we can get rid of --gadget-namespace in case when there is only one gadget daemonset deployed.
I will take a deeper look at the code later, as it changed a bit with regard to the last time I checked it.

cmd/kubectl-gadget/utils/flags.go

burak-ok · 2023-11-30T09:40:34Z

I am nonetheless still wondering if we can get rid of --gadget-namespace in case when there is only one gadget daemonset deployed.

I think that makes sense now. This PR is (after the PR split) looking at only a single namespace.

I will take a deeper look at the code later, as it changed a bit with regard to the last time I checked it.

The code changes are mostly moving the gadgetNamespace around. So adding it as parameter and argument

burak-ok · 2023-11-30T10:52:58Z

I've added 2 commits:

Add k8s-app=gadget label to the namespace itself. I think in general this is a good idea, too
Detect the deployed gadget namespace. This information comes from the added namespace labels. The detection is NOT done for deploy, undeploy and version (Is version okay?)

cmd/kubectl-gadget/conf-flags/k8s-config-flags.go

eiffel-fl

I tested it again and traceloop is broken:

$ ./kubectl-gadget traceloop start           (remotes/origin/burak/custom_namespace) %
Error: getting traces by gadget name: the server could not find the requested resource (get traces.gadget.kinvolk.io)
...
$ ./kubectl-gadget traceloop start --gadget-namespace foobar
Error: getting traces by gadget name: the server could not find the requested resource (get traces.gadget.kinvolk.io)

cmd/kubectl-gadget/main.go

cmd/kubectl-gadget/conf-flags/k8s-config-flags.go

eiffel-fl · 2023-11-30T11:14:14Z

Also the auto-detect feature for undeploy seems to have problems:

$ ./kubectl-gadget undeploy                  (remotes/origin/burak/custom_namespace) %
Removing traces...
Removing CRD...
Removing cluster role binding...
Removing cluster role...
INFO[0000] Nothing to do, gadget namespace doesn't exist 
Inspektor Gadget successfully removed
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ ../kubectl get ns                          (remotes/origin/burak/custom_namespace) %
NAME              STATUS   AGE
default           Active   15d
foobar            Active   17h
``

burak-ok · 2023-11-30T11:16:27Z

Also the auto-detect feature for undeploy seems to have problems:

For deploy (doesn't make sense anyway) and undeploy I think the namespace should be given explicitly. That is why it doesn't work for them

burak-ok · 2023-11-30T11:20:52Z

I tested it again and traceloop is broken:

$ ./kubectl-gadget traceloop start           (remotes/origin/burak/custom_namespace) %
Error: getting traces by gadget name: the server could not find the requested resource (get traces.gadget.kinvolk.io)
...
$ ./kubectl-gadget traceloop start --gadget-namespace foobar
Error: getting traces by gadget name: the server could not find the requested resource (get traces.gadget.kinvolk.io)

For me it seems to work:

{12:19}~/codes/inspektor-gadget:burak/custom_namespace ✓ ➭ go run ./cmd/kubectl-gadget/... traceloop start                      
{12:19}~/codes/inspektor-gadget:burak/custom_namespace ✓ ➭ go run ./cmd/kubectl-gadget/... traceloop list 
NODE                               NAMESPACE                          POD                                CONTAINER                          CONTAINERID       
minikube-docker                    kube-system                        kube-proxy-ncdnv                   kube-proxy                         7277fae5f697dbeec6
minikube-docker                    kube-system                        kube-scheduler-minikube-docker     kube-scheduler                     5a8c7a138b26f51fbe
minikube-docker                    kube-system                        storage-provisioner                storage-provisioner                7c1715cd2b585b5ce2
minikube-docker                    kubernetes-dashboard               dashboard-metrics…64c9f7bf9d-wkqcx dashboard-metrics-scraper          11e766a729b5bd2e0f
minikube-docker                    foo                                gadget-xddb4                       gadget                             22583f5a846b46b01c
minikube-docker                    kube-system                        coredns-6d4b75cb6d-cl58f           coredns                            3f923089585b896dbe
minikube-docker                    kube-system                        kube-controller-m…-minikube-docker kube-controller-manager            00cec3f4a7bb825ea9
minikube-docker                    kube-system                        etcd-minikube-docker               etcd                               275ecf5102e475a514
minikube-docker                    kube-system                        kube-apiserver-minikube-docker     kube-apiserver                     e86ca2a364bf8f5de1
minikube-docker                    kubernetes-dashboard               kubernetes-dashbo…-7fd776546-xqsc7 kubernetes-dashboard               a31fe8814ffb424dbc

eiffel-fl

I tested it again and traceloop is broken:

$ ./kubectl-gadget traceloop start           (remotes/origin/burak/custom_namespace) %
Error: getting traces by gadget name: the server could not find the requested resource (get traces.gadget.kinvolk.io)
...
$ ./kubectl-gadget traceloop start --gadget-namespace foobar
Error: getting traces by gadget name: the server could not find the requested resource (get traces.gadget.kinvolk.io)

For me it seems to work:

{12:19}~/codes/inspektor-gadget:burak/custom_namespace ✓ ➭ go run ./cmd/kubectl-gadget/... traceloop start                      
{12:19}~/codes/inspektor-gadget:burak/custom_namespace ✓ ➭ go run ./cmd/kubectl-gadget/... traceloop list 
NODE                               NAMESPACE                          POD                                CONTAINER                          CONTAINERID       
minikube-docker                    kube-system                        kube-proxy-ncdnv                   kube-proxy                         7277fae5f697dbeec6
minikube-docker                    kube-system                        kube-scheduler-minikube-docker     kube-scheduler                     5a8c7a138b26f51fbe
minikube-docker                    kube-system                        storage-provisioner                storage-provisioner                7c1715cd2b585b5ce2
minikube-docker                    kubernetes-dashboard               dashboard-metrics…64c9f7bf9d-wkqcx dashboard-metrics-scraper          11e766a729b5bd2e0f
minikube-docker                    foo                                gadget-xddb4                       gadget                             22583f5a846b46b01c
minikube-docker                    kube-system                        coredns-6d4b75cb6d-cl58f           coredns                            3f923089585b896dbe
minikube-docker                    kube-system                        kube-controller-m…-minikube-docker kube-controller-manager            00cec3f4a7bb825ea9
minikube-docker                    kube-system                        etcd-minikube-docker               etcd                               275ecf5102e475a514
minikube-docker                    kube-system                        kube-apiserver-minikube-docker     kube-apiserver                     e86ca2a364bf8f5de1
minikube-docker                    kubernetes-dashboard               kubernetes-dashbo…-7fd776546-xqsc7 kubernetes-dashboard               a31fe8814ffb424dbc

For strange reasons, I did not have the trace CRD installed.
Now everything is OK:

$ ./kubectl-gadget traceloop show 1c6bbb5fe7c3e80 | head
INFO[0000] Experimental features enabled                
CPU PID              COMM             SYSCALL            PARAMS                                   RET
1   24347            coredns          epoll_pwait        epfd=4, events=0xc000151840, maxevents=… 0  
1   24347            coredns          futex              uaddr=0xc000600150, op=129, val=1, utim… 1  
1   24347            coredns          futex              uaddr=0x3087338, op=129, val=1, utime=0… 1  
1   24347            coredns          recvmsg            fd=15, msg=0xc0005a89f8, flags=0         -1…
6   24347            coredns          epoll_pwait        epfd=4, events=0xc00018d838, maxevents=… 0  
1   24347            coredns          epoll_pwait        epfd=4, events=0xc000151840, maxevents=… 0  
6   24347            coredns          epoll_pwait        epfd=4, events=0xc00018d838, maxevents=… 0  
1   24347            coredns          futex              uaddr=0xc000100950, op=128, val=0, utim… 0  
3   24347            coredns          nanosleep          rqtp=0xc000085f10, rmtp=0x0              0  
$ ./kubectl-gadget undeploy --gadget-namespace foobar
INFO[0000] Experimental features enabled                
Removing traces...
Removing traces...
Removing CRD...
Removing cluster role binding...
Removing cluster role...
Removing namespace...
Waiting for namespace to be removed...
Inspektor Gadget successfully removed

I just have one nit regarding the package name, otherwise we are ready to go.

pkg/runtime/grpc/grpc-runtime.go

cmd/kubectl-gadget/utils/flags.go

cmd/kubectl-gadget/advise/advise.go

pkg/resources/manifests/deploy.yaml

cmd/kubectl-gadget/utils/k8s.go

burak-ok · 2023-12-01T16:19:02Z

#2223 was merged, please take into account this comment: #2223 (comment)

If everyone is okay I would fix the hardcoded namespace for --pull-secret in a follow up PR, for the following reasons:

This PR would get stalled longer

Everything under kubectl gadget run is experimental

The use case for using kubectl gadget run and --pull-secret at the same time is even smaller

You know... When you plan something it will not work.
This time i planned to not support --pull-secret so in the end it is now supported 🎉

It was easier once the dependency from grpcruntime to cmd/kubectl-gadget/... was solved.

I kept them as separate commits for easier reviewing

eiffel-fl

We need to test the helm path, but the kubectl-gadget one is OK:

francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ ../kubectl create ns foobar                (remotes/origin/burak/custom_namespace) %
namespace/foobar created
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ ../kubectl create secret docker-registry gadget-pull-secret -n foobar --from-file=.dockerconfigjson=$HOME/.docker/config.json
secret/gadget-pull-secret created
$ ./kubectl-gadget deploy --gadget-namespace foobar --image-pull-policy Never
INFO[0000] Experimental features enabled                
Creating Namespace/foobar...
Creating ServiceAccount/gadget...
Creating ClusterRole/gadget-cluster-role...
Creating ClusterRoleBinding/gadget-cluster-role-binding...
Creating Role/gadget-role...
Creating RoleBinding/gadget-role-binding...
Creating DaemonSet/gadget...
Creating CustomResourceDefinition/traces.gadget.kinvolk.io...
Waiting for gadget pod(s) to be ready...
0/1 gadget pod(s) ready
1/1 gadget pod(s) ready
Retrieving Gadget Catalog...
Inspektor Gadget successfully deployed
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ ./kubectl-gadget trace exec -A | head -5    (remotes/origin/burak/custom_namespace) %
INFO[0000] Experimental features enabled                
K8S.NODE                       K8S.NAMESPACE                  K8S.POD                        K8S.CONTAINER                  PID              PPID             COMM             RET ARGS                                    
minikube                       kube-system                    kube-proxy-db7l5               kube-proxy                     4562             2455             iptables         0   /usr/sbin/iptables -w 5 -W 100000 -S KU…
minikube                       foobar                         gadget-4vwzn                   gadget                         4573             4563             gadgettracerman  0   /bin/gadgettracermanager -liveness      
minikube                       foobar                         gadget-4vwzn                   gadget                         4595             4583             gadgettracerman  0   /bin/gadgettracermanager -liveness      
minikube                       foobar                         gadget-4vwzn                   gadget                         4628             4619             gadgettracerman  0   /bin/gadgettracermanager -liveness
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ ./kubectl-gadget traceloop start           (remotes/origin/burak/custom_namespace) %
INFO[0000] Experimental features enabled                
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ ./kubectl-gadget traceloop list            (remotes/origin/burak/custom_namespace) %
INFO[0000] Experimental features enabled                
NODE                           NAMESPACE                      POD                           CONTAINER                     CONTAINERID    
minikube                       kube-system                    kube-scheduler-minikube       kube-scheduler                dcce8858394287c
minikube                       kube-system                    storage-provisioner           storage-provisioner           4c7d6bf1e5af42e
minikube                       foobar                         gadget-4vwzn                  gadget                        d7d52f4db34d7c4
minikube                       kube-system                    coredns-565d847f94-ldtwx      coredns                       849e3ef033a9d80
minikube                       kube-system                    etcd-minikube                 etcd                          5426c5da87c6c4b
minikube                       kube-system                    kube-apiserver-minikube       kube-apiserver                45a1cc45bc5e3ee
minikube                       kube-system                    kube-controlle…nager-minikube kube-controller-manager       928e8a235ac31dc
minikube                       kube-system                    kube-proxy-db7l5              kube-proxy                    e0b437a2e0d90c6
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ ./kubectl-gadget traceloop show dcce8858394287c | head -5
INFO[0000] Experimental features enabled                
CPU PID              COMM             SYSCALL            PARAMS                                   RET
0   1835             kube-scheduler   epoll_pwait        epfd=4, events=0xc000087858, maxevents=… 0  
0   1835             kube-scheduler   epoll_pwait        epfd=4, events=0xc000087858, maxevents=… 0  
1   1835             kube-scheduler   nanosleep          rqtp=0xc000085f10, rmtp=0x0              0  
1   1835             kube-scheduler   futex              uaddr=0xc000436948, op=129, val=1, utim… 1
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ ./kubectl-gadget run francisregistry.azurecr.io/sockets -A | head        
INFO[0000] Experimental features enabled                
K8S.NODE                       K8S.NAMESPACE                  K8S.POD                        K8S.CONTAINER                  SRC              DST             
minikube                                                                                                                    r/127.0.0.1:984  r/0.0.0.0:0     
minikube                                                                                                                    r/0.0.0.0:42065  r/0.0.0.0:0     
minikube                                                                                                                    r/:::5355        r/:::0          
minikube                                                                                                                    r/0.0.0.0:5355   r/0.0.0.0:0     
minikube                                                                                                                    r/0.0.0.0:38751  r/0.0.0.0:0     
minikube                                                                                                                    r/:::48213       r/:::0          
minikube                                                                                                                    r/:::56477       r/:::0          
minikube                                                                                                                    r/:::52540       r/:::0          
minikube                                                                                                                    r/:::60844       r/:::0

I am nonetheless not really sure about the gadget output, but it works.

eiffel-fl · 2023-12-01T16:12:57Z

pkg/gadgets/runner.go

@@ -26,6 +26,7 @@ type GadgetContext interface {
 	ID() string
 	Context() context.Context
 	GadgetParams() *params.Params
+	RuntimeParams() *params.Params


I do not understand why is this changed in this particular commit?

That is because of Line 116 in file pkg/gadgets/run/tracer/tracer.go of the same commit

cmd/kubectl-gadget/main.go

eiffel-fl · 2023-12-01T16:15:46Z

docs/install.md

+By default Inspektor Gadget is deployed to the namespace `gadget`.
+This can be changed with the `--gadget-namespace` flag.
+When using gadgets (e.g. `kubectl gadget trace exec`) the deployed namespace is discovered automatically and no additional flags are needed during the usage.
+For `undeploy` the `--gadget-namespace` flag is mandatory.


Some documentation for helm chart would also be welcomed.

Any reason for this requirement? I think it'll make sense also for this command to autodetect the namespace.

I consider undeploy a "dangerous" operation where the user should be rather explicit about his intentions. There is no technical reason for this though.

Maybe I am also thinking about the next PR (parallel Inspektor Gadget instances). There a undeploy from a team shouldn't silently undeploy an Inspektor Gadget instance of another team if that one is currently the only one running.

I am also skeptical about using undeploy with custom namespace since we delete the namespace. What if I deploy it in kube-system namespace and run undeploy. It will try to delete the kube-system namespace? Perhaps we should only delete the namespace if we created it.

Maybe an additional flag for --keep-namespace too?

I am also skeptical about using undeploy with custom namespace since we delete the namespace

Good point. I wasn't considering that it was possible to deploy IG to an existing namespace.

Perhaps we should only delete the namespace if we created it.

Yeah. Perhaps having adding an annotation would help.

Maybe an additional flag for --keep-namespace too?

I don't think we need a flag like that. IMO we should limit a lot the new flags we add.

(We can handle this in a following PR, it's fine for me to require the gadget-namespace parameter in the undeploy command.)

burak-ok · 2023-12-01T16:37:20Z

We need to test the helm path

For simple local tests I did the following:

Modify charts/Makefile and replaced --namespace gadget with --namespace foobar

{17:33}~/codes/inspektor-gadget:burak/custom_namespace ✗ ➭ make -C charts install  
make: Entering directory '/home/burak/codes/inspektor-gadget/charts'
rm -rf bin
Building chart:
mkdir -p bin
cp -r gadget bin
cp -r ../pkg/resources/crd/bases bin/gadget/crds
mv bin/gadget/Chart.yaml.tmpl bin/gadget/Chart.yaml
sed -i "s/%VERSION%/"1.0.0-dev"/g" bin/gadget/Chart.yaml
sed -i "s/%APP_VERSION%/burak-custom_namespace/g" bin/gadget/Chart.yaml
Preparing docs
docker run --user 1000:1000 -v /home/burak/codes/inspektor-gadget/charts/bin/gadget:/helm-docs jnorwood/helm-docs:v1.11.0 -s file
time="2023-12-01T16:33:53Z" level=info msg="Found Chart directories [.]"
time="2023-12-01T16:33:53Z" level=info msg="Generating README Documentation for chart ."
Charts available at: bin/gadget
helm upgrade --install gadget bin/gadget --namespace foobar --create-namespace
Release "gadget" does not exist. Installing it now.
NAME: gadget
LAST DEPLOYED: Fri Dec  1 17:33:53 2023
NAMESPACE: foobar
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
Inspektor Gadget "burak-custom_namespace" installed successfully!

For usage details, visit https://www.inspektor-gadget.io
kubectl apply -f bin/gadget/crds/*
Warning: resource customresourcedefinitions/traces.gadget.kinvolk.io is missing the kubectl.kubernetes.io/last-applied-configuration annotation which is required by kubectl apply. kubectl apply should only be used on resources created declaratively by either kubectl create --save-config or kubectl apply. The missing annotation will be patched automatically.
customresourcedefinition.apiextensions.k8s.io/traces.gadget.kinvolk.io configured
make: Leaving directory '/home/burak/codes/inspektor-gadget/charts'

{17:33}~/codes/inspektor-gadget:burak/custom_namespace ✗ ➭ go run ./cmd/kubectl-gadget/... trace tcp
K8S.NODE                K8S.NAMESPACE           K8S.POD                K8S.CONTAINER          T PID        COMM        IP SRC                           DST                          
minikube-docker         default                 test-pod               test-pod               C 882707     wget        4  p/default/test-pod:34288      r/1.1.1.1:80                 
minikube-docker         default                 test-pod               test-pod               X 882707     wget        4  p/default/test-pod:34288      r/1.1.1.1:80                 
minikube-docker         default                 test-pod               test-pod               C 882707     wget        4  p/default/test-pod:55664      r/1.1.1.1:443                
^C%                                                                                                                                                                                  

{17:34}~/codes/inspektor-gadget:burak/custom_namespace ✗ ➭ go run ./cmd/kubectl-gadget/... trace tcp --gadget-namespace foobar
K8S.NODE                K8S.NAMESPACE           K8S.POD                K8S.CONTAINER          T PID        COMM        IP SRC                           DST                          
minikube-docker         default                 test-pod               test-pod               C 883126     wget        4  p/default/test-pod:51524      r/1.1.1.1:80                 
minikube-docker         default                 test-pod               test-pod               X 883126     wget        4  p/default/test-pod:51524      r/1.1.1.1:80                 
minikube-docker         default                 test-pod               test-pod               C 883126     wget        4  p/default/test-pod:56442      r/1.1.1.1:443                
^C%                                                                                                                                                                                  

{17:34}~/codes/inspektor-gadget:burak/custom_namespace ✗ ➭ make -C charts uninstall                                           
make: Entering directory '/home/burak/codes/inspektor-gadget/charts'
kubectl delete -f bin/gadget/crds/*
customresourcedefinition.apiextensions.k8s.io "traces.gadget.kinvolk.io" deleted
helm uninstall gadget --namespace foobar
release "gadget" uninstalled
make: Leaving directory '/home/burak/codes/inspektor-gadget/charts'

It seems to work

I am nonetheless not really sure about the gadget output, but it works.

Which output do you mean?

Signed-off-by: Burak Ok <burakok@microsoft.com>

mauriciovasquezbernal

Some comments.

cmd/kubectl-gadget/advise/seccomp-profile.go

pkg/gadgets/run/tracer/run.go

pkg/gadgets/run/tracer/tracer.go

cmd/kubectl-gadget/utils/k8s.go

cmd/kubectl-gadget/undeploy.go

cmd/kubectl-gadget/deploy.go

pkg/runtime/grpc/grpc-runtime.go

mauriciovasquezbernal · 2023-12-01T19:40:40Z

docs/install.md

+By default Inspektor Gadget is deployed to the namespace `gadget`.
+This can be changed with the `--gadget-namespace` flag.
+When using gadgets (e.g. `kubectl gadget trace exec`) the deployed namespace is discovered automatically and no additional flags are needed during the usage.
+For `undeploy` the `--gadget-namespace` flag is mandatory.


Any reason for this requirement? I think it'll make sense also for this command to autodetect the namespace.

pkg/runtime/grpc/grpc-runtime.go

cmd/kubectl-gadget/main.go

eiffel-fl · 2023-12-04T10:39:18Z

pkg/gadgets/run/tracer/run.go

+		// TODO: Namespace is still hardcoded
+		secretBytes, err = getPullSecret(pullSecretString, "gadget")


What is the status on this? It will be done in a follow-up PR?

Unfortunately yes, this will be another PR.
Shouldn't have tried to support that in a hurry

cmd/kubectl-gadget/main.go

eiffel-fl

I renew my approval as this PR permits to deploy Inspektor Gadget in a specific namespace.
I tested with helm on top of b1f02a8 and it works:

francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ ../kubectl create ns foobar               (remotes/origin/burak/custom_namespace) *%
namespace/foobar created
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ ../kubectl create secret docker-registry gadget-pull-secret -n foobar --from-file=.dockerconfigjson=$HOME/.docker/config.json
secret/gadget-pull-secret created
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ git diff                                  (remotes/origin/burak/custom_namespace) *%
diff --git a/charts/Makefile b/charts/Makefile
index d4828b7b..0c5235a3 100644
--- a/charts/Makefile
+++ b/charts/Makefile
@@ -28,7 +28,7 @@ build: clean
 # install uses 'helm upgrade --install' to make chart installation idempotent,
 # also helm upgrade does not update CRDs so we need to apply them separately
 install: build
-       $(HELM) upgrade --install gadget $(CHART_DIR) --namespace gadget --create-namespace
+       $(HELM) upgrade --install gadget $(CHART_DIR) --namespace foobar --create-namespace
        kubectl apply -f $(CHART_DIR)/crds/*
 
 uninstall:
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ make -C charts build                      (remotes/origin/burak/custom_namespace) *%
make : on entre dans le répertoire « /home/francis/Codes/kinvolk/inspektor-gadget/charts »
Charts available at: bin/gadget
make : on quitte le répertoire « /home/francis/Codes/kinvolk/inspektor-gadget/charts »
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ helm install -n foobar gadget --set config.mountPullSecret=true --set image.pullPolicy=Never --set config.experimental=true  ./charts/bin/gadget/
...
Inspektor Gadget "HEAD" installed successfully!

For usage details, visit https://www.inspektor-gadget.io
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ IG_EXPERIMENTAL=true ./kubectl-gadget run francisregistry.azurecr.io/sockets -A | head -3
INFO[0000] Experimental features enabled                
K8S.NODE                       K8S.NAMESPACE                  K8S.POD                        K8S.CONTAINER                  SRC              DST             
minikube                       kube-system                    coredns-565d847f94-ldtwx       coredns                        r/:::8080        r/:::0          
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ ./kubectl-gadget traceloop start          (remotes/origin/burak/custom_namespace) *%
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ ./kubectl-gadget traceloop list           (remotes/origin/burak/custom_namespace) *%
NODE                           NAMESPACE                      POD                           CONTAINER                     CONTAINERID    
minikube                       kube-system                    kube-controlle…nager-minikube kube-controller-manager       5ddd688a875417e
minikube                       foobar                         gadget-djzzg                  gadget                        75ae5e79bae7d7b
minikube                       kube-system                    kube-proxy-db7l5              kube-proxy                    d2f9a4a9623a4a6
minikube                       kube-system                    kube-scheduler-minikube       kube-scheduler                f939318aa7deb73
minikube                       kube-system                    storage-provisioner           storage-provisioner           69756dd3a51d035
minikube                       kube-system                    coredns-565d847f94-ldtwx      coredns                       4b9de7869205dff
minikube                       kube-system                    etcd-minikube                 etcd                          3925b1c854c56dc
minikube                       kube-system                    kube-apiserver-minikube       kube-apiserver                6db3e6ab8d7e716
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ ./kubectl-gadget traceloop show 3925b1c854c56dc | head -5
CPU PID              COMM             SYSCALL            PARAMS                                   RET
1   1620             etcd             futex                                                       1  
1   1620             etcd             futex                                                       0  
1   1620             etcd             epoll_pwait        epfd=4, events=0xc000065800, maxevents=… 0  
1   1620             etcd             epoll_pwait        epfd=4, events=0xc000065800, maxevents=… 0

I just do not really understand how the secret stuff was able to work as it is still hardcoded here.

cmd/kubectl-gadget/main.go

eiffel-fl · 2023-12-04T11:14:24Z

cmd/kubectl-gadget/main.go

+		if len(gadgetNamespaces) == 0 {
+			if !isVersion {
+				log.Fatalf("No running Inspektor Gadget instances found")
+			}
+		} else if len(gadgetNamespaces) > 1 {
+			log.Fatalf("Multiple running Inspektor Gadget instances found in following namespaces: %v", gadgetNamespaces)
+		} else {
+			runtimeGlobalParams.Set(grpcruntime.ParamGadgetNamespace, gadgetNamespaces[0])
+		}


Suggested change

if len(gadgetNamespaces) == 0 {

if !isVersion {

log.Fatalf("No running Inspektor Gadget instances found")

}

} else if len(gadgetNamespaces) > 1 {

log.Fatalf("Multiple running Inspektor Gadget instances found in following namespaces: %v", gadgetNamespaces)

} else {

runtimeGlobalParams.Set(grpcruntime.ParamGadgetNamespace, gadgetNamespaces[0])

}

if len(gadgetNamespaces) == 0 && !isVersion {

log.Fatalf("No running Inspektor Gadget instances found")

}

if len(gadgetNamespaces) > 1 {

log.Fatalf("Multiple running Inspektor Gadget instances found in following namespaces: %v", gadgetNamespaces)

}

runtimeGlobalParams.Set(grpcruntime.ParamGadgetNamespace, gadgetNamespaces[0])

Since Fatalf() is equivalent to call Exit(), you can use the above.

Ah yes, you are right. But the runtimeGlobalParams.Set(grpcruntime.ParamGadgetNamespace, gadgetNamespaces[0]) still has to be done only if there are any namespaces in gadgetNamespaces. Therefore for that I need an else

Suggested change

if len(gadgetNamespaces) == 0 {

if !isVersion {

log.Fatalf("No running Inspektor Gadget instances found")

}

} else if len(gadgetNamespaces) > 1 {

log.Fatalf("Multiple running Inspektor Gadget instances found in following namespaces: %v", gadgetNamespaces)

} else {

runtimeGlobalParams.Set(grpcruntime.ParamGadgetNamespace, gadgetNamespaces[0])

}

switch len(gadgetNamespaces) > 1 {

case 0:

if !isVersion {

log.Fatalf("No running Inspektor Gadget instances found")

}

case 1:

runtimeGlobalParams.Set(grpcruntime.ParamGadgetNamespace, gadgetNamespaces[0])

default:

log.Fatalf("Multiple running Inspektor Gadget instances found in following namespaces: %v", gadgetNamespaces)

}

A switch would then do the trick.

mauriciovasquezbernal

Some minor comments but IMO it can be merged.

One thing that is not working is trying to deploy to the same namespace twice:
It works fine for the gadget namespace:

# deploy first time
$ ./kubectl-gadget deploy
Creating Namespace/gadget...
Creating ServiceAccount/gadget...
Creating ClusterRole/gadget-cluster-role...
Creating ClusterRoleBinding/gadget-cluster-role-binding...
Creating Role/gadget-role...
Creating RoleBinding/gadget-role-binding...
Creating DaemonSet/gadget...
Creating CustomResourceDefinition/traces.gadget.kinvolk.io...
Waiting for gadget pod(s) to be ready...
0/1 gadget pod(s) ready
1/1 gadget pod(s) ready
Retrieving Gadget Catalog...
Inspektor Gadget successfully deployed

# try to deploy again
$ ./kubectl-gadget deploy
Creating Namespace/gadget...
Creating ServiceAccount/gadget...
Creating ClusterRole/gadget-cluster-role...
Creating ClusterRoleBinding/gadget-cluster-role-binding...
Creating Role/gadget-role...
Creating RoleBinding/gadget-role-binding...
Creating DaemonSet/gadget...
The gadget pod(s) weren't modified!

it doesn't work with a custom namespace:

$ ./kubectl-gadget deploy --gadget-namespace=xyz
Creating Namespace/xyz...
Creating ServiceAccount/gadget...
Creating ClusterRole/gadget-cluster-role...
Creating ClusterRoleBinding/gadget-cluster-role-binding...
Creating Role/gadget-role...
Creating RoleBinding/gadget-role-binding...
Creating DaemonSet/gadget...
Creating CustomResourceDefinition/traces.gadget.kinvolk.io...
Waiting for gadget pod(s) to be ready...
1/1 gadget pod(s) ready
Retrieving Gadget Catalog...
Inspektor Gadget successfully deployed

# hangs until timeout happens
$ ./kubectl-gadget deploy --gadget-namespace=xyz
Creating Namespace/xyz...
Creating ServiceAccount/gadget...
Creating ClusterRole/gadget-cluster-role...
Creating ClusterRoleBinding/gadget-cluster-role-binding...
Creating Role/gadget-role...
Creating RoleBinding/gadget-role-binding...
Creating DaemonSet/gadget...
Creating CustomResourceDefinition/traces.gadget.kinvolk.io...
Waiting for gadget pod(s) to be ready...
Error: timed out waiting for the condition

pkg/gadgets/run/tracer/tracer.go

mauriciovasquezbernal · 2023-12-04T12:47:33Z

docs/install.md

+By default Inspektor Gadget is deployed to the namespace `gadget`.
+This can be changed with the `--gadget-namespace` flag.
+When using gadgets (e.g. `kubectl gadget trace exec`) the deployed namespace is discovered automatically and no additional flags are needed during the usage.
+For `undeploy` the `--gadget-namespace` flag is mandatory.


I am also skeptical about using undeploy with custom namespace since we delete the namespace

Good point. I wasn't considering that it was possible to deploy IG to an existing namespace.

Perhaps we should only delete the namespace if we created it.

Yeah. Perhaps having adding an annotation would help.

Maybe an additional flag for --keep-namespace too?

I don't think we need a flag like that. IMO we should limit a lot the new flags we add.

(We can handle this in a following PR, it's fine for me to require the gadget-namespace parameter in the undeploy command.)

mauriciovasquezbernal · 2023-12-04T12:50:04Z

cmd/kubectl-gadget/deploy.go

+		if len(gadgetNamespaces) != 0 {
+			if len(gadgetNamespaces) == 1 && gadgetNamespaces[0] == gadgetNamespace {
+				// Idempotent, this behavior is allowed
+			} else {
+				// Inspektor Gadget is the program name and therefore capitalized (Lint error ST1005)
+				//nolint:all
+				return fmt.Errorf("Inspektor Gadget is already deployed to the following namespaces: %v. Only a single instance is allowed", gadgetNamespaces)
+			}
+		}
+	}


I think it's better to write this in a different way to avoid an empty else:

if len(gadgetNamespaces) != 0 && gadgetNamespaces[0] != gadgetNamespace { // Inspektor Gadget is the program name and therefore capitalized (Lint error ST1005) //nolint:all return fmt.Errorf("Inspektor Gadget is already deployed to the following namespaces: %v. Only a single instance is allowed", gadgetNamespaces) }

IMO the intention is clearer with the empty if/else

The suggested code would sometimes not complain in the following situation:
We have 2 deployed clusters: 1. foo and 2. bar

Now if we do deploy --gadget-namespace foo the behavior is dependent on the ordering of the namespaces we get back from the K8s API call.

If the array has foo as its first element, it doesn't complain (gadgetNamespaces[0] != gadgetNamespace). But if the first element is bar we get the Error message

IMO the intention is clearer with the empty if/else

I did not catch this code while reviewing but I share Mauricio's opinion.
Having an empty if sounds strange and I do not know plenty of codes depicting this.

We have 2 deployed clusters: 1. foo and 2. bar

For the moment, we only allow having one and it would ease everything to think with only one.

Whereas our deploy will not allow creating a new IG instance while one is running, we do not have any control over helm charts.
Nonetheless I will change it to the suggested code

Signed-off-by: Burak Ok <burakok@microsoft.com>

This is accomplished through moving the K8s secret reading codepath into the run tracer. The raw bytes that the secret stored is then given as a parameter to the oci pkg Signed-off-by: Burak Ok <burakok@microsoft.com>

burak-ok · 2023-12-04T13:42:00Z

One thing that is not working is trying to deploy to the same namespace twice: It works fine for the gadget namespace:

Fixed. When searching for the DaemonSet in deploy.go I didn't search for the name gadget in the namespace xyz. Instead I searched for xyz in the namespace xyz. Is fixed now.

After the CI is finished running I will finally merge then

Signed-off-by: Burak Ok <burakok@microsoft.com>

burak-ok requested review from flyth, mauriciovasquezbernal and alban as code owners November 27, 2023 13:10

burak-ok linked an issue Nov 27, 2023 that may be closed by this pull request

[RFE] Add support for namespace configuration for Inspektor Gadget itself #2205

Closed

burak-ok mentioned this pull request Nov 27, 2023

Allow running Inspektor Gadget in parallel namespaces #2208

Open

6 tasks

mauriciovasquezbernal reviewed Nov 28, 2023

View reviewed changes

pkg/runtime/grpc/grpc-runtime.go Outdated Show resolved Hide resolved

cmd/kubectl-gadget/main.go Show resolved Hide resolved

cmd/kubectl-gadget/main.go Outdated Show resolved Hide resolved

eiffel-fl reviewed Nov 29, 2023

View reviewed changes

burak-ok force-pushed the burak/custom_namespace branch 2 times, most recently from bb7c641 to ea35a74 Compare November 29, 2023 11:16

eiffel-fl reviewed Nov 29, 2023

View reviewed changes

cmd/kubectl-gadget/utils/flags.go Outdated Show resolved Hide resolved

cmd/kubectl-gadget/utils/trace.go Outdated Show resolved Hide resolved

mauriciovasquezbernal reviewed Nov 29, 2023

View reviewed changes

pkg/runtime/grpc/grpc-runtime.go Outdated Show resolved Hide resolved

cmd/kubectl-gadget/main.go Show resolved Hide resolved

burak-ok force-pushed the burak/custom_namespace branch 2 times, most recently from f84e3a9 to 82d85da Compare November 29, 2023 17:00

burak-ok force-pushed the burak/custom_namespace branch from 82d85da to 7af6689 Compare November 29, 2023 17:04

eiffel-fl reviewed Nov 29, 2023

View reviewed changes

cmd/kubectl-gadget/utils/flags.go Outdated Show resolved Hide resolved

burak-ok force-pushed the burak/custom_namespace branch from 7af6689 to 13b1e89 Compare November 30, 2023 10:50

burak-ok force-pushed the burak/custom_namespace branch from 13b1e89 to 8ca7f82 Compare November 30, 2023 10:56

eiffel-fl reviewed Nov 30, 2023

View reviewed changes

cmd/kubectl-gadget/conf-flags/k8s-config-flags.go Outdated Show resolved Hide resolved

burak-ok force-pushed the burak/custom_namespace branch from 8ca7f82 to c72da06 Compare November 30, 2023 11:01

eiffel-fl reviewed Nov 30, 2023

View reviewed changes

cmd/kubectl-gadget/main.go Show resolved Hide resolved

cmd/kubectl-gadget/conf-flags/k8s-config-flags.go Outdated Show resolved Hide resolved

eiffel-fl approved these changes Nov 30, 2023

View reviewed changes

burak-ok force-pushed the burak/custom_namespace branch 2 times, most recently from 8c980cd to 38495d8 Compare November 30, 2023 14:20

mauriciovasquezbernal reviewed Nov 30, 2023

View reviewed changes

burak-ok force-pushed the burak/custom_namespace branch from 4694ec1 to 6f62692 Compare December 1, 2023 16:16

eiffel-fl reviewed Dec 1, 2023

View reviewed changes

burak-ok force-pushed the burak/custom_namespace branch from 6f62692 to 44189e2 Compare December 1, 2023 16:43

burak-ok added 3 commits December 1, 2023 20:05

consts: Remove unused const

279aa09

Signed-off-by: Burak Ok <burakok@microsoft.com>

runtime/grpc: Decouple from cmd/kubectl-gadget

d45a981

Signed-off-by: Burak Ok <burakok@microsoft.com>

kubectl-gadget: Move init() to NewTraceloopCommand

a7aac9e

Signed-off-by: Burak Ok <burakok@microsoft.com>

burak-ok force-pushed the burak/custom_namespace branch from 44189e2 to fba4787 Compare December 1, 2023 19:06

mauriciovasquezbernal reviewed Dec 1, 2023

View reviewed changes

burak-ok force-pushed the burak/custom_namespace branch 2 times, most recently from 12aadaa to b1f02a8 Compare December 4, 2023 10:09

eiffel-fl reviewed Dec 4, 2023

View reviewed changes

eiffel-fl approved these changes Dec 4, 2023

View reviewed changes

eiffel-fl reviewed Dec 4, 2023

View reviewed changes

cmd/kubectl-gadget/main.go Outdated Show resolved Hide resolved

eiffel-fl reviewed Dec 4, 2023

View reviewed changes

burak-ok force-pushed the burak/custom_namespace branch 2 times, most recently from d0d19d2 to 4acc577 Compare December 4, 2023 12:33

mauriciovasquezbernal approved these changes Dec 4, 2023

View reviewed changes

burak-ok added 4 commits December 4, 2023 14:39

kubectl-gadget: Allow using IG in a specified namespace

fd1702e

Signed-off-by: Burak Ok <burakok@microsoft.com>

charts: Use Release.Namespace as the namespace

a3a3ea9

Signed-off-by: Burak Ok <burakok@microsoft.com>

charts: Fix gadget.fullname to "gadget"

affd8c6

Signed-off-by: Burak Ok <burakok@microsoft.com>

pkg/oci: Remove dependency on k8s package

1573fce

This is accomplished through moving the K8s secret reading codepath into the run tracer. The raw bytes that the secret stored is then given as a parameter to the oci pkg Signed-off-by: Burak Ok <burakok@microsoft.com>

burak-ok force-pushed the burak/custom_namespace branch from 4acc577 to 345d57e Compare December 4, 2023 13:39

burak-ok added 2 commits December 4, 2023 14:44

kubectl-gadget: Detect the deployed gadget namespace automatically

561e633

Signed-off-by: Burak Ok <burakok@microsoft.com>

docs: Add subchapter for flag --gadget-namespace

73f4edf

Signed-off-by: Burak Ok <burakok@microsoft.com>

burak-ok force-pushed the burak/custom_namespace branch from 345d57e to 73f4edf Compare December 4, 2023 13:44

burak-ok merged commit 8083af5 into main Dec 4, 2023
50 checks passed

burak-ok deleted the burak/custom_namespace branch December 4, 2023 14:10

		// TODO: Namespace is still hardcoded
		secretBytes, err = getPullSecret(pullSecretString, "gadget")

Allow IG to deploy into a custom namespace #2250

Allow IG to deploy into a custom namespace #2250

Conversation

burak-ok commented Nov 27, 2023

Allow IG to deploy into a custom namespace

mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

eiffel-fl left a comment

Choose a reason for hiding this comment

eiffel-fl left a comment • edited

Choose a reason for hiding this comment

burak-ok commented Nov 29, 2023 • edited

eiffel-fl left a comment

Choose a reason for hiding this comment

burak-ok commented Nov 30, 2023

burak-ok commented Nov 30, 2023

eiffel-fl left a comment

Choose a reason for hiding this comment

eiffel-fl commented Nov 30, 2023

burak-ok commented Nov 30, 2023 • edited

burak-ok commented Nov 30, 2023

eiffel-fl left a comment

Choose a reason for hiding this comment

burak-ok commented Dec 1, 2023

eiffel-fl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

burak-ok commented Dec 1, 2023 • edited

mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eiffel-fl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

burak-ok Dec 4, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

burak-ok Dec 4, 2023 • edited

Choose a reason for hiding this comment

burak-ok commented Dec 4, 2023

eiffel-fl left a comment •

edited

burak-ok commented Nov 29, 2023 •

edited

burak-ok commented Nov 30, 2023 •

edited

burak-ok commented Dec 1, 2023 •

edited

burak-ok Dec 4, 2023 •

edited

burak-ok Dec 4, 2023 •

edited