Skip to content

Commit

Permalink
Port more of Use tests
Browse files Browse the repository at this point in the history
  • Loading branch information
tomasaschan committed May 13, 2024
1 parent a99e5c3 commit dc226b6
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 29 deletions.
26 changes: 19 additions & 7 deletions pkg/envtest/setup/env/assets.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,39 @@ var expectedExecutables = []string{

// 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.Concrete, bool) {
func (e *Env) TryUseAssetsFromPath(ctx context.Context, spec versions.Spec, path string) (versions.Spec, bool) {
v, err := versions.FromPath(path)
if err != nil {
log.FromContext(ctx).Error(err, "Unable to use assets from path, ignoring", "path", path)
return nil, false
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 nil, false
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 nil, false
return versions.Spec{}, false
} else if !ok {
log.FromContext(ctx).Error(nil, "Assets path is missing some executables, ignoring", "path", path)
return nil, false
return versions.Spec{}, false
}

return v, true
return versions.Spec{Selector: v}, true
}

func (e *Env) hasAllExecutables(path string) (bool, error) {
Expand Down
10 changes: 5 additions & 5 deletions pkg/envtest/setup/env/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ import (
// or nil if no such version is available.
//
// Note that a nil error does not guarantee that a version was found!
func (e *Env) SelectLocalVersion(ctx context.Context, spec versions.Spec, platform versions.Platform) (*versions.Concrete, error) {
func (e *Env) SelectLocalVersion(ctx context.Context, spec versions.Spec, platform versions.Platform) (store.Item, error) {
localVersions, err := e.Store.List(ctx, store.Filter{Version: spec, Platform: platform})
if err != nil {
return nil, err
return store.Item{}, err
}
// NB(tomasaschan): this assumes the following of the contract for store.List
// * only results matching the provided filter are returned
Expand All @@ -23,11 +23,11 @@ func (e *Env) SelectLocalVersion(ctx context.Context, spec versions.Spec, platfo
// we want, and there's no need to iterate through the items again.
if len(localVersions) > 0 {
// copy to avoid holding on to the entire slice
result := localVersions[0].Version
return &result, nil
result := localVersions[0]
return result, nil
}

return nil, nil
return store.Item{}, nil
}

// PathTo returns the local path to the assets directory for the provided version and platform
Expand Down
18 changes: 9 additions & 9 deletions pkg/envtest/setup/testhelpers/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,17 @@ func initializeFakeStore(fs afero.Afero, dir string) {
}

ginkgo.By("making some fake non-store paths")
gomega.Expect(fs.Mkdir(filepath.Join(dir, "missing-binaries"), 0755)).To(gomega.Succeed())
gomega.Expect(fs.Mkdir(filepath.Join(dir, "missing", "binaries"), 0755)).To(gomega.Succeed())

gomega.Expect(fs.Mkdir(filepath.Join(dir, "wrong-version"), 0755)).To(gomega.Succeed())
gomega.Expect(fs.WriteFile(filepath.Join(dir, "wrong-version", "kube-apiserver"), nil, 0755)).To(gomega.Succeed())
gomega.Expect(fs.WriteFile(filepath.Join(dir, "wrong-version", "kubectl"), nil, 0755)).To(gomega.Succeed())
gomega.Expect(fs.WriteFile(filepath.Join(dir, "wrong-version", "etcd"), nil, 0755)).To(gomega.Succeed())
gomega.Expect(fs.Mkdir(filepath.Join(dir, "wrong", "version"), 0755)).To(gomega.Succeed())
gomega.Expect(fs.WriteFile(filepath.Join(dir, "wrong", "version", "kube-apiserver"), nil, 0755)).To(gomega.Succeed())
gomega.Expect(fs.WriteFile(filepath.Join(dir, "wrong", "version", "kubectl"), nil, 0755)).To(gomega.Succeed())
gomega.Expect(fs.WriteFile(filepath.Join(dir, "wrong", "version", "etcd"), nil, 0755)).To(gomega.Succeed())

gomega.Expect(fs.Mkdir(filepath.Join(dir, "good-version"), 0755)).To(gomega.Succeed())
gomega.Expect(fs.WriteFile(filepath.Join(dir, "good-version", "kube-apiserver"), nil, 0755)).To(gomega.Succeed())
gomega.Expect(fs.WriteFile(filepath.Join(dir, "good-version", "kubectl"), nil, 0755)).To(gomega.Succeed())
gomega.Expect(fs.WriteFile(filepath.Join(dir, "good-version", "etcd"), nil, 0755)).To(gomega.Succeed())
gomega.Expect(fs.Mkdir(filepath.Join(dir, "a", "good", "version"), 0755)).To(gomega.Succeed())
gomega.Expect(fs.WriteFile(filepath.Join(dir, "a", "good", "version", "kube-apiserver"), nil, 0755)).To(gomega.Succeed())
gomega.Expect(fs.WriteFile(filepath.Join(dir, "a", "good", "version", "kubectl"), nil, 0755)).To(gomega.Succeed())
gomega.Expect(fs.WriteFile(filepath.Join(dir, "a", "good", "version", "etcd"), nil, 0755)).To(gomega.Succeed())
// TODO: put the right files
}

Expand Down
16 changes: 8 additions & 8 deletions pkg/envtest/setup/use/use.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ import (
"fmt"

"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 summarizes the output of the Use workflow
type Result struct {
Version versions.Concrete
Version versions.Spec
Platform versions.Platform
MD5 string
Path string
Expand All @@ -29,7 +30,7 @@ func Use(ctx context.Context, version versions.Spec, options ...Option) (Result,
if cfg.assetPath != "" {
if v, ok := env.TryUseAssetsFromPath(ctx, version, cfg.assetPath); ok {
return Result{
Version: *v,
Version: v,
Platform: cfg.platform,
Path: cfg.assetPath,
}, nil
Expand All @@ -39,13 +40,12 @@ func Use(ctx context.Context, version versions.Spec, options ...Option) (Result,
if !cfg.forceDownload && !version.CheckLatest {
if selected, err := env.SelectLocalVersion(ctx, version, cfg.platform); err != nil {
return Result{}, err
} else if selected != nil {
} else if selected != (store.Item{}) {
return Result{
Path: env.PathTo(selected, cfg.platform),
Version: *selected,
Platform: cfg.platform,
Path: env.PathTo(&selected.Version, selected.Platform),
Version: versions.Spec{Selector: selected.Version},
Platform: selected.Platform,
}, nil

}
}

Expand All @@ -63,7 +63,7 @@ func Use(ctx context.Context, version versions.Spec, options ...Option) (Result,
}

return Result{
Version: *selectedVersion,
Version: versions.Spec{Selector: *selectedVersion},
Platform: selectedPlatform.Platform,
Path: env.PathTo(selectedVersion, selectedPlatform.Platform),
MD5: selectedPlatform.MD5,
Expand Down
141 changes: 141 additions & 0 deletions pkg/envtest/setup/use/use_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
package use_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/env"
"sigs.k8s.io/controller-runtime/pkg/envtest/setup/remote"
"sigs.k8s.io/controller-runtime/pkg/envtest/setup/testhelpers"
"sigs.k8s.io/controller-runtime/pkg/envtest/setup/use"
"sigs.k8s.io/controller-runtime/pkg/envtest/setup/versions"
)

var (
testLog logr.Logger
ctx context.Context
)

func TestEnv(t *testing.T) {
testLog = testhelpers.GetLogger()
ctx = logr.NewContext(context.Background(), testLog)

RegisterFailHandler(Fail)
RunSpecs(t, "Use Suite")
}

var _ = Describe("Use", func() {
var (
envOpts []env.Option
version = versions.Spec{
Selector: versions.Concrete{Major: 1, Minor: 16, Patch: 0},
}
)
JustBeforeEach(func() {
addr, shutdown, err := testhelpers.NewServer()
Expect(err).NotTo(HaveOccurred())
DeferCleanup(shutdown)

s := testhelpers.NewMockStore()

envOpts = append(
envOpts,
env.WithClient(&remote.Client{
Log: testLog.WithName("test-remote-client"),
Bucket: "kubebuilder-tools-test",
Server: addr,
Insecure: true,
}),
env.WithStore(s),
env.WithFS(afero.NewIOFS(s.Root)),
)
})

Context("when useEnv is set", func() {
It("should fall back to normal behavior when the env is not set", func() {
result, err := use.Use(
ctx,
version,
use.WithAssetsFromEnv(true),
use.WithPlatform("*", "*"),
use.WithEnvOptions(envOpts...),
)

Expect(err).NotTo(HaveOccurred())

Expect(result.Version).To(Equal(version))
Expect(result.Path).To(HaveSuffix("/1.16.0-linux-amd64"), "should fall back to a local version")
})

It("should fall back to normal behavior if binaries are missing", func() {
result, err := use.Use(
ctx,
version,
use.WithAssetsFromEnv(true),
use.WithAssetsAt(".test-binaries/missing-binaries"),
use.WithPlatform("*", "*"),
use.WithEnvOptions(envOpts...),
)

Expect(err).NotTo(HaveOccurred())

Expect(result.Version).To(Equal(version), "should fall back to a local version")
Expect(result.Path).To(HaveSuffix("/1.16.0-linux-amd64"))
})

It("should use the value of the env if it contains the right binaries", func() {
result, err := use.Use(
ctx,
version,
use.WithAssetsFromEnv(true),
use.WithAssetsAt("a/good/version"),
use.WithPlatform("*", "*"),
use.WithEnvOptions(envOpts...),
)

Expect(err).NotTo(HaveOccurred())

Expect(result.Version).To(Equal(versions.AnyVersion))
Expect(result.Path).To(HaveSuffix("/good/version"))
})

It("should not try to check the version of the binaries", func() {
result, err := use.Use(
ctx,
version,
use.WithAssetsFromEnv(true),
use.WithAssetsAt("wrong/version"),
use.WithPlatform("*", "*"),
use.WithEnvOptions(envOpts...),
)

Expect(err).NotTo(HaveOccurred())

Expect(result.Version).To(Equal(versions.AnyVersion))
Expect(result.Path).To(Equal("wrong/version"))
})

It("should not need to contact the network", func() {
envOpts = append(envOpts, env.WithClient(nil)) // ensure tests fail if we try to contact remote

result, err := use.Use(
ctx,
version,
use.WithAssetsFromEnv(true),
use.WithAssetsAt("a/good/version"),
use.WithPlatform("*", "*"),
use.WithEnvOptions(envOpts...),
)

Expect(err).NotTo(HaveOccurred())

Expect(result.Version).To(Equal(versions.AnyVersion))
Expect(result.Path).To(HaveSuffix("/good/version"))
})
})
})

0 comments on commit dc226b6

Please sign in to comment.