kubernetes-sigs / kubebuilder-declarative-pattern

A toolkit for building declarative operators with kubebuilder
Apache License 2.0
256 stars 85 forks source link

feat: avoid importing kubectl deps when using ApplysetApplier #331

Closed yuwenma closed 1 year ago

yuwenma commented 1 year ago

What this PR does / why we need it: This PR allows k-d-p operators to not import the kubectl dependent libraries if using ApplysetApplier.

CI change and test coverage: Some Test files are tagged with without_kubectl_deps specifically. We added a new CI test run in the dev/test to make sure those tests are run with the without_kubectl_deps tag

CGO_ENABLED=0 go test -tags without_kubectl_deps -count=1 -v ./...

usage:

Take Guestbook as an example,

// examples/guestbook-operator/controllers/guestbook_controller.go
applier := applier.NewApplySetApplier(metav1.PatchOptions{}, metav1.DeleteOptions{}, applier.ApplysetOptions{})
return r.Reconciler.Init(mgr, &api.Guestbook{},
        ...
        declarative.WithApplier(applier),
        declarative.WithApplyPrune())

go run -tags without_kubectl_deps main.go

yuwenma commented 1 year ago

/assign @justinsb

yuwenma commented 1 year ago

Looks good, but I think if we can avoid go:build without_kubectl_deps except in one file that builds the default object, I think this PR gets simpler.

qq: but I think this will cause the without_kubectl_deps test to call the test cases that need the kubectl dep (which cause undefined error because the direct.go and exec.go are excluded). e.g. run go test -tags without_direct_applier,without_exec_applier will include test with_kubectl_test.go:

// assuming I commented out the tags
//go:build !without_exec_applier || !without_direct_applier
// +build !without_exec_applier !without_direct_applier

func TestSimpleReconciler(t *testing.T) {
    Key := "direct"
    t.Run(Key, func(t *testing.T) {
        testharness.RunGoldenTests(t, "testdata/reconcile/"+Key+"/", func(h *testharness.Harness, testdir string) {
            // ! NewDirectApplier raises undeclared error
            testSimpleReconciler(h, testdir, applier.NewDirectApplier(), status.NewBasic(nil)) 
        })
    })
}
justinsb commented 1 year ago

Thanks @yuwenma - lgtm!

/approve /lgtm

k8s-ci-robot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justinsb, yuwenma

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/kubebuilder-declarative-pattern/blob/master/OWNERS)~~ [justinsb] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment