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.

Sign up for GitHub

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

Jump to bottom

CI: Auto update inspektor-gadget YAMLs #19717

Merged
merged 2 commits into from
Oct 9, 2024
Merged

CI: Auto update inspektor-gadget YAMLs #19717

merged 2 commits into from
Oct 9, 2024

Conversation

spowelljr
Copy link
Member

@spowelljr spowelljr commented Sep 27, 2024 •
edited
Loading

Auto update inspektor-gadget YAMLs

Tested:

$ minikube start --addons=inspektor-gadget
😄  minikube v1.34.0 on Darwin 14.7 (arm64)
✨  Automatically selected the docker driver. Other choices: qemu2, ssh, vfkit (experimental)
📌  Using Docker Desktop driver with root privileges
👍  Starting "minikube" primary control-plane node in "minikube" cluster
🚜  Pulling base image v0.0.45-1727108449-19696 ...
🔥  Creating docker container (CPUs=2, Memory=4000MB) ...
❗  Failing to connect to https://registry.k8s.io/ from inside the minikube container
💡  To pull new external images, you may need to configure a proxy: https://minikube.sigs.k8s.io/docs/reference/networking/proxy/
🐳  Preparing Kubernetes v1.31.1 on Docker 27.3.1 ...
    ▪ Generating certificates and keys ...
    ▪ Booting up control plane ...
    ▪ Configuring RBAC rules ...
🔗  Configuring bridge CNI (Container Networking Interface) ...
🔎  Verifying Kubernetes components...
    ▪ Using image gcr.io/k8s-minikube/storage-provisioner:v5
    ▪ Using image ghcr.io/inspektor-gadget/inspektor-gadget:v0.32.0
🌟  Enabled addons: default-storageclass, storage-provisioner, inspektor-gadget
🏄  Done! kubectl is now configured to use "minikube" cluster and "default" namespace by default

$ kubectl get pods -A
NAMESPACE     NAME                               READY   STATUS    RESTARTS   AGE
gadget        gadget-zml6g                       1/1     Running   0          50s
kube-system   coredns-7c65d6cfc9-lkf62           1/1     Running   0          50s
kube-system   etcd-minikube                      1/1     Running   0          56s
kube-system   kube-apiserver-minikube            1/1     Running   0          56s
kube-system   kube-controller-manager-minikube   1/1     Running   0          56s
kube-system   kube-proxy-8m48q                   1/1     Running   0          50s
kube-system   kube-scheduler-minikube            1/1     Running   0          56s
kube-system   storage-provisioner                1/1     Running   0          54s

cc @eiffel-fl

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 27, 2024
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 27, 2024
@spowelljr spowelljr mentioned this pull request Sep 27, 2024
Merged
eiffel-fl
eiffel-fl reviewed Sep 30, 2024
View reviewed changes
Copy link
Contributor

@eiffel-fl eiffel-fl left a comment

Choose a reason for hiding this comment

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

Hi!

Thank you! This definitely eases the whole process!
I tested it and it works perfectly:

$ go run ./cmd/minikube/ start --addons=inspektor-gadget
😄  minikube v0.0.0-unset sur Ubuntu 22.04
🎉  minikube 1.34.0 est disponible ! Téléchargez-le ici : https://github.com/kubernetes/minikube/releases/tag/v1.34.0
💡  Pour désactiver cette notification, exécutez : 'minikube config set WantUpdateNotification false'

✨  Choix automatique du pilote docker. Autres choix: kvm2, qemu2, ssh
📌  Utilisation du pilote Docker avec le privilège root
👍  Démarrage du nœud "minikube" primary control-plane dans le cluster "minikube"
🚜  Extraction de l'image de base v0.0.45-1727108449-19696...
🔥  Création de docker container (CPU=2, Memory=7900Mo) ...
...
🌟  Modules activés: default-storageclass, inspektor-gadget
💡  kubectl introuvable. Si vous en avez besoin, essayez : 'minikube kubectl -- get pods -A'
🏄  Terminé ! kubectl est maintenant configuré pour utiliser "minikube" cluster et espace de noms "default" par défaut.
$ ../kubectl get pod -n gadget                  (remotes/spowelljr/autoUpdateInspekYAML) %
NAME           READY   STATUS    RESTARTS   AGE
gadget-qhh7w   1/1     Running   0          2m42s

I only found one nit, but I am not sure about it.

Best regards.

hack/update/inspektor_gadget_version/update_inspektor_gadget_version.go
klog.Fatalf("failed to read body: %v", err)
}
replacements := map[string]string{
`ghcr\.io\/inspektor-gadget\/inspektor-gadget:.*`: "{{.CustomRegistries.InspektorGadget | default .ImageRepository | default .Registries.InspektorGadget }}{{.Images.InspektorGadget}}",
Copy link
Contributor

Choose a reason for hiding this comment

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

I am wondering if we should not just replace ``ghcr.io/inspektor-gadget/inspektor-gadget`.
This way, it would also update the corresponding environment value:
bbeb1da#diff-6cba2efa346e98239fde3c0ac5bad4058aa50bd90c1da8d4042efda77cf1af0dL77

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for pointing this out, the problem is that {{.Images.InspektorGadget}} includes the image and the tag, where there seems to be a specific env for the image and tag individually. This in itself if not a big issue because I could make two new map fields InspektorGadgetWithoutVersion and InspektorGadgetVersion (or something similar). But this is further complicated that we allow the user to overwrite the .Images.InspektorGadget using flags. So then the envs and the image actually used would be out of sync again. Just curious, what are the GADGET_IMAGE & INSPEKTOR_GADGET_VERSION envs used for.

Copy link
Contributor

Choose a reason for hiding this comment

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

They are only used for logging purpose:
https://github.com/inspektor-gadget/inspektor-gadget/blob/df184595c61021d87a3a82c02e4ac965a707edaf/gadget-container/entrypoint/entrypoint.go#L250
https://github.com/inspektor-gadget/inspektor-gadget/blob/df184595c61021d87a3a82c02e4ac965a707edaf/gadget-container/entrypoint/entrypoint.go#L264

@medyagh
Copy link
Member

medyagh commented Oct 9, 2024

/lgtm

@medyagh medyagh merged commit 48cd795 into kubernetes:master Oct 9, 2024
26 of 29 checks passed
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 9, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: medyagh, spowelljr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eiffel-fl eiffel-fl eiffel-fl left review comments

@ComradeProgrammer ComradeProgrammer Awaiting requested review from ComradeProgrammer

@medyagh medyagh Awaiting requested review from medyagh

Assignees

@medyagh medyagh

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@spowelljr @medyagh @k8s-ci-robot @eiffel-fl