-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Re-implement the setup-envtest functionality in a package
There are two main points of re-implementing vs just moving the code: 1. Error handling in the old code base was based on panicking and recovering, where the recover basically just read out a field from the panic value and determined the correct exit code. In a package, we want more nuanced error handling, especially in order to allow test suites to catch the errors and surface them through their own reporting mechanisms. 2. There was a lot of global state in the old code base, "hidden" in the env.Env type that was used as a receiver for all the methods. This re-implementation tries to make the state more explicit, keeping only dependencies (like the remote client and local store) in the environment, while retaining the same behavior as the previous implementation. Tests have been ported over to their respective workflow sub-packages, and a few new ones have been added to cover cases the old test suite for one reason or another did not. Thus, we can be fairly confident that the new implementation does not break old functionality, even if it is a significant rewrite.
- Loading branch information
1 parent
1f5b39f
commit c8aa9b3
Showing
55 changed files
with
2,389 additions
and
1,985 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| package cleanup | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
|
|
||
| "sigs.k8s.io/controller-runtime/pkg/envtest/setup/env" | ||
| "sigs.k8s.io/controller-runtime/pkg/envtest/setup/store" | ||
| "sigs.k8s.io/controller-runtime/pkg/envtest/setup/versions" | ||
| ) | ||
|
|
||
| // Result is a list of version-platform pairs that were removed from the store. | ||
| type Result []store.Item | ||
|
|
||
| // Cleanup removes binary packages from disk for all version-platform pairs that match the parameters | ||
| // | ||
| // Note that both the item collection and the error might be non-nil, if some packages were successfully | ||
| // removed (they will be listed in the first return value) and some failed (the errors will be collected | ||
| // in the second). | ||
| func Cleanup(ctx context.Context, spec versions.Spec, options ...Option) (Result, error) { | ||
| cfg := configure(options...) | ||
|
|
||
| env, err := env.New(cfg.envOpts...) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if err := env.Store.Initialize(ctx); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| items, err := env.Store.Remove(ctx, store.Filter{Version: spec, Platform: cfg.platform}) | ||
| if errors.Is(err, store.ErrUnableToList) { | ||
| return nil, err | ||
| } | ||
|
|
||
| // store.Remove returns an error if _any_ item failed to be removed, | ||
| // but it also reports any items that were removed without errors. | ||
| // Therefore, both items and err might be non-nil at the same time. | ||
| return items, err | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| package cleanup_test | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| . "github.com/onsi/ginkgo/v2" | ||
| . "github.com/onsi/gomega" | ||
| "github.com/spf13/afero" | ||
|
|
||
| "github.com/go-logr/logr" | ||
| "sigs.k8s.io/controller-runtime/pkg/envtest/setup/cleanup" | ||
| "sigs.k8s.io/controller-runtime/pkg/envtest/setup/env" | ||
| "sigs.k8s.io/controller-runtime/pkg/envtest/setup/store" | ||
| "sigs.k8s.io/controller-runtime/pkg/envtest/setup/testhelpers" | ||
| "sigs.k8s.io/controller-runtime/pkg/envtest/setup/versions" | ||
| ) | ||
|
|
||
| var ( | ||
| testLog logr.Logger | ||
| ctx context.Context | ||
| ) | ||
|
|
||
| func TestCleanup(t *testing.T) { | ||
| testLog = testhelpers.GetLogger() | ||
| ctx = logr.NewContext(context.Background(), testLog) | ||
|
|
||
| RegisterFailHandler(Fail) | ||
| RunSpecs(t, "Cleanup Suite") | ||
| } | ||
|
|
||
| var _ = Describe("Cleanup", func() { | ||
| var ( | ||
| defaultEnvOpts []env.Option | ||
| s *store.Store | ||
| ) | ||
|
|
||
| BeforeEach(func() { | ||
| s = testhelpers.NewMockStore() | ||
| }) | ||
|
|
||
| JustBeforeEach(func() { | ||
| defaultEnvOpts = []env.Option{ | ||
| env.WithClient(nil), // ensures we fail if we try to connect | ||
| env.WithStore(s), | ||
| env.WithFS(afero.NewIOFS(s.Root)), | ||
| } | ||
| }) | ||
|
|
||
| Context("when cleanup is run", func() { | ||
| version := versions.Spec{ | ||
| Selector: versions.Concrete{ | ||
| Major: 1, | ||
| Minor: 16, | ||
| Patch: 1, | ||
| }, | ||
| } | ||
|
|
||
| var ( | ||
| matching, nonMatching []store.Item | ||
| ) | ||
|
|
||
| BeforeEach(func() { | ||
| // ensure there are some versions matching what we're about to delete | ||
| var err error | ||
| matching, err = s.List(ctx, store.Filter{Version: version, Platform: versions.Platform{OS: "linux", Arch: "amd64"}}) | ||
| Expect(err).NotTo(HaveOccurred()) | ||
| Expect(matching).NotTo(BeEmpty(), "found no matching versions before cleanup") | ||
|
|
||
| // ensure there are some versions _not_ matching what we're about to delete | ||
| nonMatching, err = s.List(ctx, store.Filter{Version: versions.Spec{Selector: versions.PatchSelector{Major: 1, Minor: 17, Patch: versions.AnyPoint}}, Platform: versions.Platform{OS: "linux", Arch: "amd64"}}) | ||
| Expect(err).NotTo(HaveOccurred()) | ||
| Expect(nonMatching).NotTo(BeEmpty(), "found no non-matching versions before cleanup") | ||
| }) | ||
|
|
||
| JustBeforeEach(func() { | ||
| Expect(cleanup.Cleanup( | ||
| ctx, | ||
| version, | ||
| cleanup.WithPlatform("linux", "amd64"), | ||
| cleanup.WithEnvOptions(defaultEnvOpts...), | ||
| )).To(Succeed()) | ||
| }) | ||
|
|
||
| It("should remove matching versions", func() { | ||
| items, err := s.List(ctx, store.Filter{Version: version, Platform: versions.Platform{OS: "linux", Arch: "amd64"}}) | ||
| Expect(err).NotTo(HaveOccurred()) | ||
| Expect(items).To(BeEmpty(), "found matching versions after cleanup") | ||
| }) | ||
|
|
||
| It("should not remove non-matching versions", func() { | ||
| items, err := s.List(ctx, store.Filter{Version: versions.AnyVersion, Platform: versions.Platform{OS: "*", Arch: "*"}}) | ||
| Expect(err).NotTo(HaveOccurred()) | ||
| Expect(items).To(ContainElements(nonMatching), "non-matching items were affected") | ||
| }) | ||
| }) | ||
| }) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| package cleanup | ||
|
|
||
| import ( | ||
| "runtime" | ||
|
|
||
| "sigs.k8s.io/controller-runtime/pkg/envtest/setup/env" | ||
| "sigs.k8s.io/controller-runtime/pkg/envtest/setup/versions" | ||
| ) | ||
|
|
||
| type config struct { | ||
| envOpts []env.Option | ||
| platform versions.Platform | ||
| } | ||
|
|
||
| // Option is a functional option for configuring the cleanup process. | ||
| type Option func(*config) | ||
|
|
||
| // WithEnvOptions adds options to the environment setup. | ||
| func WithEnvOptions(opts ...env.Option) Option { | ||
| return func(cfg *config) { | ||
| cfg.envOpts = append(cfg.envOpts, opts...) | ||
| } | ||
| } | ||
|
|
||
| // WithPlatform sets the platform to use for cleanup. | ||
| func WithPlatform(os string, arch string) Option { | ||
| return func(cfg *config) { | ||
| cfg.platform = versions.Platform{OS: os, Arch: arch} | ||
| } | ||
| } | ||
|
|
||
| func configure(options ...Option) *config { | ||
| cfg := &config{ | ||
| platform: versions.Platform{ | ||
| Arch: runtime.GOARCH, | ||
| OS: runtime.GOOS, | ||
| }, | ||
| } | ||
|
|
||
| for _, opt := range options { | ||
| opt(cfg) | ||
| } | ||
|
|
||
| return cfg | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| package env | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
| "io/fs" | ||
| "path/filepath" | ||
|
|
||
| "sigs.k8s.io/controller-runtime/pkg/envtest/setup/versions" | ||
| "sigs.k8s.io/controller-runtime/pkg/log" | ||
| ) | ||
|
|
||
| var expectedExecutables = []string{ | ||
| "kube-apiserver", | ||
| "etcd", | ||
| "kubectl", | ||
| } | ||
|
|
||
| // TryUseAssetsFromPath attempts to use the assets from the provided path if they match the spec. | ||
| // If they do not, or if some executable is missing, it returns an empty string. | ||
| func (e *Env) TryUseAssetsFromPath(ctx context.Context, spec versions.Spec, path string) (versions.Spec, bool) { | ||
| v, err := versions.FromPath(path) | ||
| if err != nil { | ||
| ok, checkErr := e.hasAllExecutables(path) | ||
| log.FromContext(ctx).Info("has all executables", "ok", ok, "err", checkErr) | ||
| if checkErr != nil { | ||
| log.FromContext(ctx).Error(errors.Join(err, checkErr), "Failed checking if assets path has all binaries, ignoring", "path", path) | ||
| return versions.Spec{}, false | ||
| } else if ok { | ||
| // If the path has all executables, we can use it even if we can't parse the version. | ||
| // The user explicitly asked for this path, so set the version to a wildcard so that | ||
| // it passes checks downstream. | ||
| return versions.AnyVersion, true | ||
| } | ||
|
|
||
| log.FromContext(ctx).Error(errors.Join(err, errors.New("some required binaries missing")), "Unable to use assets from path, ignoring", "path", path) | ||
| return versions.Spec{}, false | ||
| } | ||
|
|
||
| if !spec.Matches(*v) { | ||
| log.FromContext(ctx).Error(nil, "Assets path does not match spec, ignoring", "path", path, "spec", spec) | ||
| return versions.Spec{}, false | ||
| } | ||
|
|
||
| if ok, err := e.hasAllExecutables(path); err != nil { | ||
| log.FromContext(ctx).Error(err, "Failed checking if assets path has all binaries, ignoring", "path", path) | ||
| return versions.Spec{}, false | ||
| } else if !ok { | ||
| log.FromContext(ctx).Error(nil, "Assets path is missing some executables, ignoring", "path", path) | ||
| return versions.Spec{}, false | ||
| } | ||
|
|
||
| return versions.Spec{Selector: v}, true | ||
| } | ||
|
|
||
| func (e *Env) hasAllExecutables(path string) (bool, error) { | ||
| for _, expected := range expectedExecutables { | ||
| _, err := e.FS.Open(filepath.Join(path, expected)) | ||
| if err != nil { | ||
| if errors.Is(err, fs.ErrNotExist) { | ||
| return false, nil | ||
| } | ||
| return false, fmt.Errorf("check for existence of %s binary in %s: %w", expected, path, err) | ||
| } | ||
| } | ||
|
|
||
| return true, nil | ||
| } |
Oops, something went wrong.