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

feat: avoid importing kubectl deps when using ApplysetApplier #331

Merged
merged 1 commit into from
May 5, 2023
Merged

feat: avoid importing kubectl deps when using ApplysetApplier #331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions dev/test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ pushd applylib
CGO_ENABLED=0 go test -count=1 -v ./...
popd

# default, test direct kubectl applier
CGO_ENABLED=0 go test -count=1 -v ./...
# test applyset applier, without kubectl direct and exec dependencies
CGO_ENABLED=0 go test -tags without_exec_applier,without_direct_applier -count=1 -v ./...
# test exec kubectl applier, without direct_applier dependencies
CGO_ENABLED=0 go test -tags without_direct_applier -count=1 -v ./...

pushd examples/guestbook-operator
CGO_ENABLED=0 go test -count=1 -v ./...
Expand Down
4 changes: 1 addition & 3 deletions pkg/patterns/declarative/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,9 +543,7 @@ func TestAddIfNotPresent(t *testing.T) {
t.Fatalf("failed to get restmapping for parent: %v", err)
}
options.ParentRef = applyset.NewParentRef(parent, "kdp-test", "default", restmapping)
applier := applier.NewApplySetApplier(
metav1.PatchOptions{FieldManager: "kdp-test"}, metav1.DeleteOptions{}, applier.ApplysetOptions{})

applier := applier.DefaultApplier
Copy link
Contributor Author

Choose a reason for hiding this comment

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

tolerant build tag. Any applier should pass this test.

if err := applier.Apply(ctx, options); err != nil {
t.Fatalf("failed to apply objects: %v", err)
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/patterns/declarative/pkg/applier/direct.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
//go:build !without_exec_applier || !without_direct_applier
// +build !without_exec_applier !without_direct_applier
Comment on lines +1 to +2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this file contains kubectl deps.


package applier

import (
Expand All @@ -6,8 +9,6 @@ import (
"os"
"strings"

"k8s.io/kubectl/pkg/util/prune"

"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
Expand All @@ -20,6 +21,7 @@ import (
"k8s.io/kubectl/pkg/cmd/apply"
cmdDelete "k8s.io/kubectl/pkg/cmd/delete"
cmdutil "k8s.io/kubectl/pkg/cmd/util"
"k8s.io/kubectl/pkg/util/prune"
"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative/pkg/manifest"
)

Expand Down
3 changes: 3 additions & 0 deletions pkg/patterns/declarative/pkg/applier/direct_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
//go:build !without_exec_applier || !without_direct_applier
// +build !without_exec_applier !without_direct_applier
Comment on lines +1 to +2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file contains kubectl deps. Tag to allow skipping importing.


/*
Copyright 2022 The Kubernetes Authors.

Expand Down
3 changes: 3 additions & 0 deletions pkg/patterns/declarative/pkg/applier/exec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
//go:build !without_exec_applier
// +build !without_exec_applier

/*
Copyright 2018 The Kubernetes Authors.

Expand Down
3 changes: 3 additions & 0 deletions pkg/patterns/declarative/pkg/applier/exec_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
//go:build !without_exec_applier
// +build !without_exec_applier
Comment on lines +1 to +2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file contains exec.go methods


/*
Copyright 2019 The Kubernetes Authors.

Expand Down
6 changes: 6 additions & 0 deletions pkg/patterns/declarative/pkg/applier/global.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
//go:build !without_exec_applier || !without_direct_applier
// +build !without_exec_applier !without_direct_applier

package applier

var DefaultApplier = NewDirectApplier()
10 changes: 10 additions & 0 deletions pkg/patterns/declarative/pkg/applier/global_without_kubectl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
//go:build without_exec_applier && without_direct_applier
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes sense, this is the one file where I would expect to see these not negated, because we want complete coverage of all the build tags in the various global_....go files.

// +build without_exec_applier,without_direct_applier

package applier

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

var DefaultApplier = NewApplySetApplier(metav1.PatchOptions{}, metav1.DeleteOptions{}, ApplysetOptions{})
2 changes: 1 addition & 1 deletion pkg/patterns/declarative/reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (e *ErrorResult) Error() string {
}

// For mocking
var defaultApplier = applier.NewDirectApplier()
var defaultApplier = applier.DefaultApplier

func (r *Reconciler) Init(mgr manager.Manager, prototype DeclarativeObject, opts ...ReconcilerOption) error {
r.prototype = prototype
Expand Down
30 changes: 0 additions & 30 deletions pkg/test/testreconciler/simpletest/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@ import (
"context"
"net/http"
"path/filepath"
"testing"
"time"

"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/rest"
Expand All @@ -19,7 +17,6 @@ import (

"sigs.k8s.io/kubebuilder-declarative-pattern/mockkubeapiserver"
"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/addon/pkg/loaders"
"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/addon/pkg/status"
"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative"
"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative/pkg/applier"
"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/test/httprecorder"
Expand All @@ -28,33 +25,6 @@ import (
api "sigs.k8s.io/kubebuilder-declarative-pattern/pkg/test/testreconciler/simpletest/v1alpha1"
)

func TestSimpleReconciler(t *testing.T) {
yuwenma marked this conversation as resolved.
Show resolved Hide resolved
appliers := []struct {
Key string
Applier applier.Applier
Status declarative.Status
}{
{
Key: "direct",
Applier: applier.NewDirectApplier(),
Status: status.NewBasic(nil),
},
{
Key: "ssa",
Applier: applier.NewApplySetApplier(
metav1.PatchOptions{FieldManager: "kdp-test"}, metav1.DeleteOptions{}, applier.ApplysetOptions{}),
Status: status.NewKstatusCheck(nil, nil),
},
}
for _, applier := range appliers {
t.Run(applier.Key, func(t *testing.T) {
testharness.RunGoldenTests(t, "testdata/reconcile/"+applier.Key+"/", func(h *testharness.Harness, testdir string) {
testSimpleReconciler(h, testdir, applier.Applier, applier.Status)
})
})
}
}

func testSimpleReconciler(h *testharness.Harness, testdir string, applier applier.Applier, status declarative.Status) {
ctx := context.Background()

Expand Down
21 changes: 21 additions & 0 deletions pkg/test/testreconciler/simpletest/with_kubectl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//go:build !without_exec_applier || !without_direct_applier
// +build !without_exec_applier !without_direct_applier

package simpletest

import (
"testing"

"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/addon/pkg/status"
"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative/pkg/applier"
"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/test/testharness"
)

func TestDirectSimpleReconciler(t *testing.T) {
Key := "direct"
t.Run(Key, func(t *testing.T) {
testharness.RunGoldenTests(t, "testdata/reconcile/"+Key+"/", func(h *testharness.Harness, testdir string) {
testSimpleReconciler(h, testdir, applier.NewDirectApplier(), status.NewBasic(nil))
})
})
}
21 changes: 21 additions & 0 deletions pkg/test/testreconciler/simpletest/without_kubectl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package simpletest

import (
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/addon/pkg/status"
"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative/pkg/applier"
"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/test/testharness"
)

func TestSSASimpleReconciler(t *testing.T) {
Key := "ssa"
a := applier.NewApplySetApplier(
metav1.PatchOptions{FieldManager: "kdp-test"}, metav1.DeleteOptions{}, applier.ApplysetOptions{})
t.Run(Key, func(t *testing.T) {
testharness.RunGoldenTests(t, "testdata/reconcile/"+Key+"/", func(h *testharness.Harness, testdir string) {
testSimpleReconciler(h, testdir, a, status.NewKstatusCheck(nil, nil))
})
})
}