feat: avoid importing kubectl deps when using ApplysetApplier

[yuwenma]

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]

/assign @justinsb

[justinsb]

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.

[justinsb]

Dumb question: why is this one needed? I would have expected only to see go:build !without_kubectl_deps

[yuwenma]

oh, because this test needs to import and use the ApplysetApplier (see code). Without this building tag, go test won't successfully import the applier package.

[justinsb]

The Applyset applier should always be present, I think.

[justinsb]

Maybe we do this by applier, because there are two "dependencies" on kubectl - there is execing kubectl, and there is calling into the kubectl code.

So maybe we have two tags: without_kubectl_applier (or without_exec_applier) and without_direct_applier ?

[yuwenma]

Sure thing! Yeah, I was thinking about to define the tags for 3 kinds rather than 2, but on my side I don't have a dedicated use case that need to distinguish exec and direct kubectl.

[yuwenma]

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]

So I would only expect to see these negated (except for globals.go). Otherwise (in this case) users must specify without_direct_applier to get the exec applier.

So I would expect to see this to be //go:build !without_exec_applier

[justinsb]

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.

[yuwenma]

tolerant build tag. Any applier should pass this test.

[yuwenma]

this file contains kubectl deps.

[yuwenma]

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

[yuwenma]

This file contains exec.go methods

[justinsb]

Thanks @yuwenma - lgtm!

/approve
/lgtm

[k8s-ci-robot]

[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 [justinsb]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment