vmware-tanzu / sonobuoy

Sonobuoy is a diagnostic tool that makes it easier to understand the state of a Kubernetes cluster by running a set of Kubernetes conformance tests and other plugins in an accessible and non-destructive manner.
https://sonobuoy.io
Apache License 2.0
2.92k stars 345 forks source link

Use image information YAML for airgap package information #1161

Closed vladimirvivien closed 3 years ago

vladimirvivien commented 4 years ago

Describe the solution you'd like Currently, the procedure to prepare a release for a new Kubernetes version involves copying/changing code from upstream K/K test package into the image package for Sonobuoy. That step, used for support of airgap operations, is documented in the release and is known to be time consuming and error prone. This practice may have evolved organically as Sonobuoy has code based has changed and the upstream K/K test packages have diverged over the years.

Originally, this issue proposed a programmatic approach to integrate with the Kubernetes/Kubernetes upstream test utility package to pull the registry and the image information dynamically instead of the manual copy/paste approach currently done. However, after researching it, it turns out using Kubernetes/Kubernetes as a programmatic dependency is almost impossible.

YAML all the things

The Kubernetes test build util currently supports a registry YAML which can be used to configure registry entries where dependent e2e test images are pulled from. Sonobuoy can use that same YAML to configure e2e image download to configure custom repositories and airgap operations.

This issue proposes that Sonobuoy uses an enhanced version of the YAML that supports configuration of both registry and image entries. That way, instead of copying code from Kubernetes upstream, the YAML file would be used to specify repositories and images to pull for e2e tests.

Example YAML

release:
  version: v1.20.0
registries:
  gcAuthenticatedRegistry: gcr.io/authenticated-image-pulling,
  dockerLibraryRegistry:   docker.io/library,
  dockerGluster:           docker.io/gluster,
  e2eRegistry:             gcr.io/kubernetes-e2e-test-images,
...
images:
- image:
  registry: gcAuthenticatedRegistry
  name: alpine
  version: 3.7
- image:
  registry: gcAuthenticatedRegistry
  name: windows-nanoserver
  version: v1
- image: 
  registry: e2eRegistry
  name: sample-apiserver
  version: 1.17
...

Implications This proposal would imply the followings, if adopted:

zubron commented 4 years ago

This is awesome research! I think it would be great to remove that manual step from the release process. I'm concerned about reducing the supported versions and having to pin the Sonobuoy version to the K8s version that you want to test. I think it could be an annoying experience for people wanting to test against multiple cluster versions - especially as this change would only be in place to improve the air-gapped use case. It would also mean that any new features/bug fixes would have to be backported to older versions so that people running Sonobuoy against older clusters can still take advantage of them. This doesn't affect the e2e plugin so much, but we've had to introduce new features for some of the other plugins that were created.

What do you think about removing all of this image handling from Sonobuoy altogether and instead creating a separate utility that is just for prepping an airgapped environment for e2e/conformance testing? That way it could have pinned versions that line up with the cluster that is being tested and it would help remove some of the (painful) coupling that exists between Sonobuoy and upstream K/K.

vladimirvivien commented 4 years ago

@zubron thanks for the awesome feedback. I failed to consider all implications you mentioned above in your comment, mainly the ability to run Sonobuoy against multiple cluster versions. That said, I think starting with then next release of Sonobuoy, the tool should rigidly apply the two-version regression limit. For instance, Sonobuoy 0.19.0 should only back support K8s 1.17, and 1.18.

As you alluded to, the portion of the code that requires this sync, during release, is for the support of airgap. I wholeheartedly think that portion should be extracted as a separate tool. This would remove the need to carry that information with each release of Kubernetes.

vladimirvivien commented 3 years ago

This can be closed as of v0.20, a different approach was taken.