Add sonobuoy images list command #1639
Conversation
There was a problem hiding this comment.
Thanks for the submission!
Just a small change requested.
I'm curious what your use case is though; the dryrun client for list doesn't do anything particularly useful since its just hardcoded values for a hardcoded version.
We could add other versions and make that work but the concern is that it can get out of sync with the actual values and creates another dependency between us and the k8s releases.
In addition, the point of the images commands in general are to pull/push them to your airgapped registry; so if you can't reach the repos to pull/list them why list them in dry-run mode at all?
cmd/sonobuoy/app/images.go
Outdated
| var client image.Client | ||
| if flags.dryRun { | ||
| client = image.DryRunClient{} | ||
| } else { | ||
| client = image.NewDockerClient() | ||
| } | ||
| version, err := getClusterVersion(flags.k8sVersion, flags.kubeconfig) | ||
| if err != nil { | ||
| errlog.LogError(err) | ||
| os.Exit(1) | ||
| } | ||
| if err := listImages(flags.plugins, version, client); err != nil { | ||
| errlog.LogError(err) | ||
| os.Exit(1) | ||
| } |
There was a problem hiding this comment.
Lets extract that logic (since its ~15 lines repeated exactly) and use it as the run method. Just have the function take the imageFlags as an argument so that you can still make it a closure around the specific set of flag values:
Run: func(cmd *cobra.Command, args []string) {
listImgs(flags)
}
|
@johnSchnake I want to automate as much as possible. The script will do like this:
The only missing part is step 3. This PR doesn't fill this need. Another flag should be added, or maybe the dry-run should not return e2e images at all. WDYT? |
|
@johnSchnake I've pushed a newer commit with a new |
|
For the systemd-logs image (and sonobuoy) you can run: I would think the dry-run flag for the e2e plugin should just cause it to not run docker at all. So for e2e it should just figure out the image location and print that (and it seems the sonobuoy image is listed as well by default too). Right now it seems to be hardcoded for the v1.19 version. As my previous comment mentioned, I dont think that provides much value. I'm just not sure why step (5) is separated out from the e2e image itself. When you run So you can get all the images you need from: Either way, adding the dryrun flag and an explicit |
And add --druy-run flag to sonobuoy images command Signed-off-by: Mathieu Parent <[email protected]>
|
@johnSchnake Good catch, I've improved the PR to only change two things:
So, the script would look like this:
Please retrigger the CI and review 🙏 . NB: Some context here. |
|
Just a bad import to fix; once that is done it looks great. Ran some manual checks locally. I'll keep an eye out for the next fixup and if everything is green I'll merge. Thanks for the submission, I think this is great. |
Signed-off-by: Mathieu Parent <[email protected]>
|
@johnSchnake Fixed the import error. Thanks! |
What this PR does / why we need it:
sonobuoy imagesprints the image list, but there is no dry-run. This PR adds asonobuoy images listcommand with dry-run flag.Which issue(s) this PR fixes
Special notes for your reviewer:
Release note: