diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000000..007ac9901c --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,81 @@ +# This file contains all available configuration options +# with their default values. + +# options for analysis running +run: + # default concurrency is a available CPU number + concurrency: 4 + + # timeout for analysis, e.g. 30s, 5m, default is 1m + timeout: 1m + + # exit code when at least one issue was found, default is 1 + issues-exit-code: 1 + + # include test files or not, default is true + tests: true + + # which dirs to skip: issues from them won't be reported; + # can use regexp here: generated.*, regexp is applied on full path; + # default value is empty list, but default dirs are skipped independently + # from this option's value (see skip-dirs-use-default). + # "/" will be replaced by current OS file path separator to properly work + # on Windows. + skip-dirs: + - vendor + + # default is true. Enables skipping of directories: + # vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ + skip-dirs-use-default: true + + # by default isn't set. If set we pass it to "go list -mod={option}". From "go help modules": + # If invoked with -mod=readonly, the go command is disallowed from the implicit + # automatic updating of go.mod described above. Instead, it fails when any changes + # to go.mod are needed. This setting is most useful to check that go.mod does + # not need updates, such as in a continuous integration and testing system. + # If invoked with -mod=vendor, the go command assumes that the vendor + # directory holds the correct copies of dependencies and ignores + # the dependency descriptions in go.mod. + modules-download-mode: vendor + + # Allow multiple parallel golangci-lint instances running. + # If false (default) - golangci-lint acquires file lock on start. + allow-parallel-runners: false + + +# output configuration options +output: + # colored-line-number|line-number|json|tab|checkstyle|code-climate|junit-xml|github-actions + # default is "colored-line-number" + format: tab + + # print lines of code with issue, default is true + print-issued-lines: true + + # print linter name in the end of issue text, default is true + print-linter-name: true + + # make issues output unique by line, default is true + uniq-by-line: true + + # add a prefix to the output file references; default is no prefix + path-prefix: "" + + # sorts results by: filepath, line and column + sort-results: false + +linters: + disable-all: true + enable: + - deadcode + - errcheck + - gofmt + - gosimple + - govet + - ineffassign + - staticcheck + - structcheck + - typecheck + - unused + - varcheck + fast: false diff --git a/Makefile b/Makefile index 8f8219f322..bb9eaa48f8 100644 --- a/Makefile +++ b/Makefile @@ -1,3 +1,6 @@ +GOLANGCI_LINT_BIN=./bin/golangci-lint +GOLANGCI_LINT_VERSION=v1.40.1 + all: build .PHONY: all @@ -21,3 +24,12 @@ verify-codegen-crds: verify-codegen: verify-codegen-crds verify: verify-codegen .PHONY: update-codegen-crds update-codegen verify-codegen-crds verify-codegen verify + + +.PHONY: lint +## Checks the code with golangci-lint +lint: $(GOLANGCI_LINT_BIN) + ./bin/golangci-lint ${V_FLAG} run --deadline=30m + +$(GOLANGCI_LINT_BIN): + curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b ./bin ${GOLANGCI_LINT_VERSION} diff --git a/cmd/image.go b/cmd/image.go index 75220a84f4..a6ace42419 100644 --- a/cmd/image.go +++ b/cmd/image.go @@ -31,5 +31,5 @@ func runImageCmd(cmd *cobra.Command, args []string) { if err != nil { klog.Fatalf("error: %v", err) } - fmt.Printf(image) + fmt.Print(image) } diff --git a/cmd/main.go b/cmd/main.go index 8845893c2e..1414798925 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -17,7 +17,7 @@ var ( func init() { klog.InitFlags(flag.CommandLine) - flag.CommandLine.Set("alsologtostderr", "true") + _ = flag.CommandLine.Set("alsologtostderr", "true") rootCmd.PersistentFlags().AddGoFlagSet(flag.CommandLine) } diff --git a/cmd/render.go b/cmd/render.go index 92b64d88e1..10cca24af8 100644 --- a/cmd/render.go +++ b/cmd/render.go @@ -31,7 +31,7 @@ func init() { } func runRenderCmd(cmd *cobra.Command, args []string) { - flag.Set("logtostderr", "true") + _ = flag.Set("logtostderr", "true") flag.Parse() if renderOpts.outputDir == "" { diff --git a/lib/resourcemerge/core.go b/lib/resourcemerge/core.go index c220882e7c..01a2a74445 100644 --- a/lib/resourcemerge/core.go +++ b/lib/resourcemerge/core.go @@ -508,16 +508,6 @@ func ensureCapabilities(modified *bool, existing *corev1.Capabilities, required } } -func setStringSliceIfSet(modified *bool, existing *[]string, required []string) { - if required == nil { - return - } - if !equality.Semantic.DeepEqual(required, *existing) { - *existing = required - *modified = true - } -} - func setStringSlice(modified *bool, existing *[]string, required []string) { if !reflect.DeepEqual(required, *existing) { *existing = required diff --git a/lib/resourcemerge/core_test.go b/lib/resourcemerge/core_test.go index 007c94d654..9939b00c33 100644 --- a/lib/resourcemerge/core_test.go +++ b/lib/resourcemerge/core_test.go @@ -33,24 +33,24 @@ func TestEnsurePodSpec(t *testing.T) { name: "remove regular containers from existing", existing: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{Name: "test"}, - corev1.Container{Name: "to-be-removed"}}}, + {Name: "test"}, + {Name: "to-be-removed"}}}, input: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{Name: "test"}}}, + {Name: "test"}}}, expectedModified: true, expected: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{Name: "test"}}}, + {Name: "test"}}}, }, { name: "remove regular and init containers from existing", existing: corev1.PodSpec{ InitContainers: []corev1.Container{ - corev1.Container{Name: "test-init"}}, + {Name: "test-init"}}, Containers: []corev1.Container{ - corev1.Container{Name: "test"}}}, + {Name: "test"}}}, input: corev1.PodSpec{}, expectedModified: true, @@ -60,7 +60,7 @@ func TestEnsurePodSpec(t *testing.T) { name: "remove init containers from existing", existing: corev1.PodSpec{ InitContainers: []corev1.Container{ - corev1.Container{Name: "test-init"}}}, + {Name: "test-init"}}}, input: corev1.PodSpec{}, expectedModified: true, @@ -70,29 +70,29 @@ func TestEnsurePodSpec(t *testing.T) { name: "append regular and init containers", existing: corev1.PodSpec{ InitContainers: []corev1.Container{ - corev1.Container{Name: "test-init-a"}}, + {Name: "test-init-a"}}, Containers: []corev1.Container{ - corev1.Container{Name: "test-a"}}}, + {Name: "test-a"}}}, input: corev1.PodSpec{ InitContainers: []corev1.Container{ - corev1.Container{Name: "test-init-a"}, - corev1.Container{Name: "test-init-b"}, + {Name: "test-init-a"}, + {Name: "test-init-b"}, }, Containers: []corev1.Container{ - corev1.Container{Name: "test-a"}, - corev1.Container{Name: "test-b"}, + {Name: "test-a"}, + {Name: "test-b"}, }, }, expectedModified: true, expected: corev1.PodSpec{ InitContainers: []corev1.Container{ - corev1.Container{Name: "test-init-a"}, - corev1.Container{Name: "test-init-b"}, + {Name: "test-init-a"}, + {Name: "test-init-b"}, }, Containers: []corev1.Container{ - corev1.Container{Name: "test-a"}, - corev1.Container{Name: "test-b"}, + {Name: "test-a"}, + {Name: "test-b"}, }, }, }, @@ -100,27 +100,27 @@ func TestEnsurePodSpec(t *testing.T) { name: "match regular and init containers", existing: corev1.PodSpec{ InitContainers: []corev1.Container{ - corev1.Container{Name: "test-init"}}, + {Name: "test-init"}}, Containers: []corev1.Container{ - corev1.Container{Name: "test"}}}, + {Name: "test"}}}, input: corev1.PodSpec{ InitContainers: []corev1.Container{ - corev1.Container{Name: "test-init"}}, + {Name: "test-init"}}, Containers: []corev1.Container{ - corev1.Container{Name: "test"}}}, + {Name: "test"}}}, expectedModified: false, expected: corev1.PodSpec{ InitContainers: []corev1.Container{ - corev1.Container{Name: "test-init"}}, + {Name: "test-init"}}, Containers: []corev1.Container{ - corev1.Container{Name: "test"}}}, + {Name: "test"}}}, }, { name: "remove limits and requests on container", existing: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "test", Resources: corev1.ResourceRequirements{ Limits: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("2m")}, @@ -131,12 +131,12 @@ func TestEnsurePodSpec(t *testing.T) { }, input: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{Name: "test"}}}, + {Name: "test"}}}, expectedModified: true, expected: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{Name: "test"}, + {Name: "test"}, }, }, }, @@ -144,7 +144,7 @@ func TestEnsurePodSpec(t *testing.T) { name: "modify limits and requests on container", existing: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "test", Resources: corev1.ResourceRequirements{ Limits: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("2m")}, @@ -155,7 +155,7 @@ func TestEnsurePodSpec(t *testing.T) { }, input: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "test", Resources: corev1.ResourceRequirements{ Limits: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("4m")}, @@ -168,7 +168,7 @@ func TestEnsurePodSpec(t *testing.T) { expectedModified: true, expected: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "test", Resources: corev1.ResourceRequirements{ Limits: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("4m")}, @@ -182,7 +182,7 @@ func TestEnsurePodSpec(t *testing.T) { name: "match limits and requests on container", existing: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "test", Resources: corev1.ResourceRequirements{ Limits: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("2m")}, @@ -193,7 +193,7 @@ func TestEnsurePodSpec(t *testing.T) { }, input: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "test", Resources: corev1.ResourceRequirements{ Limits: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("2m")}, @@ -206,7 +206,7 @@ func TestEnsurePodSpec(t *testing.T) { expectedModified: false, expected: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "test", Resources: corev1.ResourceRequirements{ Limits: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("2m")}, @@ -220,12 +220,12 @@ func TestEnsurePodSpec(t *testing.T) { name: "add limits and requests on container", existing: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{Name: "test"}, + {Name: "test"}, }, }, input: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "test", Resources: corev1.ResourceRequirements{ Limits: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("2m")}, @@ -238,7 +238,7 @@ func TestEnsurePodSpec(t *testing.T) { expectedModified: true, expected: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "test", Resources: corev1.ResourceRequirements{ Limits: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("2m")}, @@ -252,20 +252,20 @@ func TestEnsurePodSpec(t *testing.T) { name: "remove a container", existing: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{Name: "test-A"}, - corev1.Container{Name: "test-B"}, + {Name: "test-A"}, + {Name: "test-B"}, }, }, input: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{Name: "test-B"}, + {Name: "test-B"}, }, }, expectedModified: true, expected: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{Name: "test-B"}, + {Name: "test-B"}, }, }, }, @@ -273,15 +273,15 @@ func TestEnsurePodSpec(t *testing.T) { name: "add ports on container", existing: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{Name: "test"}, + {Name: "test"}, }, }, input: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "test", Ports: []corev1.ContainerPort{ - corev1.ContainerPort{ContainerPort: 8080}, + {ContainerPort: 8080}, }, }, }, @@ -289,10 +289,10 @@ func TestEnsurePodSpec(t *testing.T) { expectedModified: true, expected: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "test", Ports: []corev1.ContainerPort{ - corev1.ContainerPort{ContainerPort: 8080}, + {ContainerPort: 8080}, }, }, }, @@ -302,20 +302,20 @@ func TestEnsurePodSpec(t *testing.T) { name: "replace ports on container", existing: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "test", Ports: []corev1.ContainerPort{ - corev1.ContainerPort{ContainerPort: 8080}, + {ContainerPort: 8080}, }, }, }, }, input: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "test", Ports: []corev1.ContainerPort{ - corev1.ContainerPort{ContainerPort: 9191}, + {ContainerPort: 9191}, }, }, }, @@ -323,10 +323,10 @@ func TestEnsurePodSpec(t *testing.T) { expectedModified: true, expected: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "test", Ports: []corev1.ContainerPort{ - corev1.ContainerPort{ContainerPort: 9191}, + {ContainerPort: 9191}, }, }, }, @@ -336,23 +336,23 @@ func TestEnsurePodSpec(t *testing.T) { name: "remove container ports", existing: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "test", Ports: []corev1.ContainerPort{ - corev1.ContainerPort{ContainerPort: 8080}, + {ContainerPort: 8080}, }, }, }, }, input: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{Name: "test"}, + {Name: "test"}, }, }, expectedModified: true, expected: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "test", Ports: []corev1.ContainerPort{}, }, diff --git a/lib/validation/validation.go b/lib/validation/validation.go index 6c38c633be..93223a15ee 100644 --- a/lib/validation/validation.go +++ b/lib/validation/validation.go @@ -69,20 +69,6 @@ func countPayloadsForVersion(config *configv1.ClusterVersion, version string) in return 0 } -func hasAmbiguousPayloadForVersion(config *configv1.ClusterVersion, version string) bool { - for _, update := range config.Status.AvailableUpdates { - if update.Version == version { - return len(update.Image) > 0 - } - } - for _, history := range config.Status.History { - if history.Version == version { - return len(history.Image) > 0 - } - } - return false -} - func ClearInvalidFields(config *configv1.ClusterVersion, errs field.ErrorList) *configv1.ClusterVersion { if len(errs) == 0 { return config diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go index af59adc857..fa36477467 100644 --- a/pkg/autoupdate/autoupdate.go +++ b/pkg/autoupdate/autoupdate.go @@ -39,8 +39,7 @@ type Controller struct { client clientset.Interface eventRecorder record.EventRecorder - syncHandler func(ctx context.Context, key string) error - statusSyncHandler func(key string) error + syncHandler func(ctx context.Context, key string) error cvLister configlistersv1.ClusterVersionLister coLister configlistersv1.ClusterOperatorLister diff --git a/pkg/cvo/cvo.go b/pkg/cvo/cvo.go index ab4d540a2f..b57a3c44ea 100644 --- a/pkg/cvo/cvo.go +++ b/pkg/cvo/cvo.go @@ -103,8 +103,6 @@ type Operator struct { payloadDir string // defaultUpstreamServer is intended for testing. defaultUpstreamServer string - // syncBackoff allows the tests to use a quicker backoff - syncBackoff wait.Backoff cvLister configlistersv1.ClusterVersionLister coLister configlistersv1.ClusterOperatorLister diff --git a/pkg/cvo/cvo_scenarios_test.go b/pkg/cvo/cvo_scenarios_test.go index bbdd7fdcf1..73743daa74 100644 --- a/pkg/cvo/cvo_scenarios_test.go +++ b/pkg/cvo/cvo_scenarios_test.go @@ -5,6 +5,7 @@ import ( "fmt" "reflect" "strconv" + "strings" "testing" "time" @@ -37,19 +38,22 @@ func setupCVOTest(payloadDir string) (*Operator, map[string]runtime.Object, *fak }) cvs := make(map[string]runtime.Object) client.AddReactor("*", "clusterversions", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { - switch a := action.(type) { - case clientgotesting.GetAction: + switch strings.ToLower(action.GetVerb()) { + case "get": + a := action.(clientgotesting.GetAction) obj, ok := cvs[a.GetName()] if !ok { return true, nil, errors.NewNotFound(schema.GroupResource{Resource: "clusterversions"}, a.GetName()) } return true, obj.DeepCopyObject(), nil - case clientgotesting.CreateAction: + case "create": + a := action.(clientgotesting.CreateAction) obj := a.GetObject().DeepCopyObject().(*configv1.ClusterVersion) obj.Generation = 1 cvs[obj.Name] = obj return true, obj, nil - case clientgotesting.UpdateAction: + case "update": + a := action.(clientgotesting.UpdateAction) obj := a.GetObject().DeepCopyObject().(*configv1.ClusterVersion) existing := cvs[obj.Name].DeepCopyObject().(*configv1.ClusterVersion) rv, _ := strconv.Atoi(existing.ResourceVersion) @@ -59,7 +63,11 @@ func setupCVOTest(payloadDir string) (*Operator, map[string]runtime.Object, *fak } else { existing.Spec = obj.Spec existing.ObjectMeta = obj.ObjectMeta - obj.Generation++ + if existing.Generation > obj.Generation { + existing.Generation = existing.Generation + 1 + } else { + existing.Generation = obj.Generation + 1 + } } existing.ResourceVersion = nextRV cvs[existing.Name] = existing @@ -195,8 +203,9 @@ func TestCVO_StartupAndSync(t *testing.T) { actual = cvs["version"].(*configv1.ClusterVersion) expectUpdateStatus(t, actions[1], "clusterversions", "", &configv1.ClusterVersion{ ObjectMeta: metav1.ObjectMeta{ - Name: "version", - Generation: 1, + Name: "version", + Generation: 1, + ResourceVersion: "1", }, Spec: configv1.ClusterVersionSpec{ ClusterID: actual.Spec.ClusterID, @@ -300,8 +309,9 @@ func TestCVO_StartupAndSync(t *testing.T) { actual = cvs["version"].(*configv1.ClusterVersion) expectUpdateStatus(t, actions[1], "clusterversions", "", &configv1.ClusterVersion{ ObjectMeta: metav1.ObjectMeta{ - Name: "version", - Generation: 1, + Name: "version", + Generation: 1, + ResourceVersion: "2", }, Spec: configv1.ClusterVersionSpec{ ClusterID: actual.Spec.ClusterID, @@ -415,7 +425,7 @@ func TestCVO_StartupAndSyncUnverifiedPayload(t *testing.T) { // make the image report unverified payloadErr := &payload.UpdateError{ Reason: "ImageVerificationFailed", - Message: fmt.Sprintf("The update cannot be verified: some random error"), + Message: "The update cannot be verified: some random error", Nested: fmt.Errorf("some random error"), } if !isImageVerificationError(payloadErr) { @@ -515,8 +525,9 @@ func TestCVO_StartupAndSyncUnverifiedPayload(t *testing.T) { actual = cvs["version"].(*configv1.ClusterVersion) expectUpdateStatus(t, actions[1], "clusterversions", "", &configv1.ClusterVersion{ ObjectMeta: metav1.ObjectMeta{ - Name: "version", - Generation: 1, + Name: "version", + Generation: 1, + ResourceVersion: "1", }, Spec: configv1.ClusterVersionSpec{ ClusterID: actual.Spec.ClusterID, @@ -620,8 +631,9 @@ func TestCVO_StartupAndSyncUnverifiedPayload(t *testing.T) { actual = cvs["version"].(*configv1.ClusterVersion) expectUpdateStatus(t, actions[1], "clusterversions", "", &configv1.ClusterVersion{ ObjectMeta: metav1.ObjectMeta{ - Name: "version", - Generation: 1, + Name: "version", + Generation: 1, + ResourceVersion: "2", }, Spec: configv1.ClusterVersionSpec{ ClusterID: actual.Spec.ClusterID, @@ -825,8 +837,9 @@ func TestCVO_StartupAndSyncPreconditionFailing(t *testing.T) { actual = cvs["version"].(*configv1.ClusterVersion) expectUpdateStatus(t, actions[1], "clusterversions", "", &configv1.ClusterVersion{ ObjectMeta: metav1.ObjectMeta{ - Name: "version", - Generation: 1, + Name: "version", + Generation: 1, + ResourceVersion: "1", }, Spec: configv1.ClusterVersionSpec{ ClusterID: actual.Spec.ClusterID, @@ -930,8 +943,9 @@ func TestCVO_StartupAndSyncPreconditionFailing(t *testing.T) { actual = cvs["version"].(*configv1.ClusterVersion) expectUpdateStatus(t, actions[1], "clusterversions", "", &configv1.ClusterVersion{ ObjectMeta: metav1.ObjectMeta{ - Name: "version", - Generation: 1, + Name: "version", + Generation: 1, + ResourceVersion: "2", }, Spec: configv1.ClusterVersionSpec{ ClusterID: actual.Spec.ClusterID, @@ -1078,7 +1092,7 @@ func TestCVO_UpgradeUnverifiedPayload(t *testing.T) { // make the image report unverified payloadErr := &payload.UpdateError{ Reason: "ImageVerificationFailed", - Message: fmt.Sprintf("The update cannot be verified: some random error"), + Message: "The update cannot be verified: some random error", Nested: fmt.Errorf("some random error"), } if !isImageVerificationError(payloadErr) { @@ -1098,7 +1112,7 @@ func TestCVO_UpgradeUnverifiedPayload(t *testing.T) { t.Fatal(err) } actions := client.Actions() - if len(actions) != 2 { + if len(actions) < 2 { t.Fatalf("%s", spew.Sdump(actions)) } @@ -1128,8 +1142,7 @@ func TestCVO_UpgradeUnverifiedPayload(t *testing.T) { expectUpdateStatus(t, actions[1], "clusterversions", "", &configv1.ClusterVersion{ ObjectMeta: metav1.ObjectMeta{ Name: "version", - ResourceVersion: "1", - Generation: 1, + ResourceVersion: "2", }, Spec: configv1.ClusterVersionSpec{ ClusterID: clusterUID, @@ -1199,7 +1212,6 @@ func TestCVO_UpgradeUnverifiedPayload(t *testing.T) { Image: "image/image:1", URL: configv1.URL("https://example.com/v1.0.1-abc"), }, - Generation: 1, }, SyncWorkerStatus{ Done: 1, @@ -1212,7 +1224,6 @@ func TestCVO_UpgradeUnverifiedPayload(t *testing.T) { URL: configv1.URL("https://example.com/v1.0.1-abc"), }, LastProgress: time.Unix(1, 0), - Generation: 1, }, SyncWorkerStatus{ Done: 2, @@ -1225,7 +1236,6 @@ func TestCVO_UpgradeUnverifiedPayload(t *testing.T) { URL: configv1.URL("https://example.com/v1.0.1-abc"), }, LastProgress: time.Unix(2, 0), - Generation: 1, }, SyncWorkerStatus{ Reconciling: true, @@ -1239,7 +1249,6 @@ func TestCVO_UpgradeUnverifiedPayload(t *testing.T) { URL: configv1.URL("https://example.com/v1.0.1-abc"), }, LastProgress: time.Unix(3, 0), - Generation: 1, }, ) client.ClearActions() @@ -1255,8 +1264,7 @@ func TestCVO_UpgradeUnverifiedPayload(t *testing.T) { expectUpdateStatus(t, actions[1], "clusterversions", "", &configv1.ClusterVersion{ ObjectMeta: metav1.ObjectMeta{ Name: "version", - ResourceVersion: "1", - Generation: 1, + ResourceVersion: "3", }, Spec: configv1.ClusterVersionSpec{ ClusterID: actual.Spec.ClusterID, @@ -1264,7 +1272,6 @@ func TestCVO_UpgradeUnverifiedPayload(t *testing.T) { DesiredUpdate: &copied, }, Status: configv1.ClusterVersionStatus{ - ObservedGeneration: 1, Desired: configv1.Release{ Version: "1.0.1-abc", Image: "image/image:1", @@ -1329,7 +1336,7 @@ func TestCVO_UpgradeUnverifiedPayloadRetrieveOnce(t *testing.T) { // make the image report unverified payloadErr := &payload.UpdateError{ Reason: "ImageVerificationFailed", - Message: fmt.Sprintf("The update cannot be verified: some random error"), + Message: "The update cannot be verified: some random error", Nested: fmt.Errorf("some random error"), } if !isImageVerificationError(payloadErr) { @@ -1349,7 +1356,7 @@ func TestCVO_UpgradeUnverifiedPayloadRetrieveOnce(t *testing.T) { t.Fatal(err) } actions := client.Actions() - if len(actions) != 2 { + if len(actions) < 2 { t.Fatalf("%s", spew.Sdump(actions)) } @@ -1379,8 +1386,7 @@ func TestCVO_UpgradeUnverifiedPayloadRetrieveOnce(t *testing.T) { expectUpdateStatus(t, actions[1], "clusterversions", "", &configv1.ClusterVersion{ ObjectMeta: metav1.ObjectMeta{ Name: "version", - ResourceVersion: "1", - Generation: 1, + ResourceVersion: "2", }, Spec: configv1.ClusterVersionSpec{ ClusterID: clusterUID, @@ -1450,7 +1456,7 @@ func TestCVO_UpgradeUnverifiedPayloadRetrieveOnce(t *testing.T) { Image: "image/image:1", URL: configv1.URL("https://example.com/v1.0.1-abc"), }, - Generation: 1, + Generation: 0, }, SyncWorkerStatus{ Done: 1, @@ -1463,7 +1469,7 @@ func TestCVO_UpgradeUnverifiedPayloadRetrieveOnce(t *testing.T) { URL: configv1.URL("https://example.com/v1.0.1-abc"), }, LastProgress: time.Unix(1, 0), - Generation: 1, + Generation: 0, }, SyncWorkerStatus{ Done: 2, @@ -1476,7 +1482,7 @@ func TestCVO_UpgradeUnverifiedPayloadRetrieveOnce(t *testing.T) { URL: configv1.URL("https://example.com/v1.0.1-abc"), }, LastProgress: time.Unix(2, 0), - Generation: 1, + Generation: 0, }, SyncWorkerStatus{ Reconciling: true, @@ -1490,7 +1496,7 @@ func TestCVO_UpgradeUnverifiedPayloadRetrieveOnce(t *testing.T) { URL: configv1.URL("https://example.com/v1.0.1-abc"), }, LastProgress: time.Unix(3, 0), - Generation: 1, + Generation: 0, }, ) client.ClearActions() @@ -1503,11 +1509,11 @@ func TestCVO_UpgradeUnverifiedPayloadRetrieveOnce(t *testing.T) { t.Fatalf("%s", spew.Sdump(actions)) } expectGet(t, actions[0], "clusterversions", "", "version") + expectUpdateStatus(t, actions[1], "clusterversions", "", &configv1.ClusterVersion{ ObjectMeta: metav1.ObjectMeta{ Name: "version", - ResourceVersion: "1", - Generation: 1, + ResourceVersion: "3", }, Spec: configv1.ClusterVersionSpec{ ClusterID: actual.Spec.ClusterID, @@ -1515,7 +1521,7 @@ func TestCVO_UpgradeUnverifiedPayloadRetrieveOnce(t *testing.T) { DesiredUpdate: &copied, }, Status: configv1.ClusterVersionStatus{ - ObservedGeneration: 1, + ObservedGeneration: 0, Desired: configv1.Release{ Version: "1.0.1-abc", Image: "image/image:1", @@ -1548,7 +1554,6 @@ func TestCVO_UpgradeUnverifiedPayloadRetrieveOnce(t *testing.T) { Image: "image/image:1", URL: configv1.URL("https://example.com/v1.0.1-abc"), }, - Generation: 1, }, SyncWorkerStatus{ Reconciling: true, @@ -1561,7 +1566,6 @@ func TestCVO_UpgradeUnverifiedPayloadRetrieveOnce(t *testing.T) { Image: "image/image:1", URL: configv1.URL("https://example.com/v1.0.1-abc"), }, - Generation: 1, }, SyncWorkerStatus{ Reconciling: true, @@ -1574,7 +1578,6 @@ func TestCVO_UpgradeUnverifiedPayloadRetrieveOnce(t *testing.T) { Image: "image/image:1", URL: configv1.URL("https://example.com/v1.0.1-abc"), }, - Generation: 1, }, SyncWorkerStatus{ Reconciling: true, @@ -1588,7 +1591,6 @@ func TestCVO_UpgradeUnverifiedPayloadRetrieveOnce(t *testing.T) { URL: configv1.URL("https://example.com/v1.0.1-abc"), }, LastProgress: time.Unix(1, 0), - Generation: 1, }, ) } @@ -1647,7 +1649,7 @@ func TestCVO_UpgradePreconditionFailing(t *testing.T) { t.Fatal(err) } actions := client.Actions() - if len(actions) != 2 { + if len(actions) < 2 { t.Fatalf("%s", spew.Sdump(actions)) } @@ -1681,8 +1683,7 @@ func TestCVO_UpgradePreconditionFailing(t *testing.T) { expectUpdateStatus(t, actions[1], "clusterversions", "", &configv1.ClusterVersion{ ObjectMeta: metav1.ObjectMeta{ Name: "version", - ResourceVersion: "1", - Generation: 1, + ResourceVersion: "2", }, Spec: configv1.ClusterVersionSpec{ ClusterID: clusterUID, @@ -1743,9 +1744,8 @@ func TestCVO_UpgradePreconditionFailing(t *testing.T) { } verifyAllStatus(t, worker.StatusCh(), SyncWorkerStatus{ - Step: "PreconditionChecks", - Actual: configv1.Release{Version: "1.0.1-abc", Image: "image/image:1"}, - Generation: 1, + Step: "PreconditionChecks", + Actual: configv1.Release{Version: "1.0.1-abc", Image: "image/image:1"}, }, SyncWorkerStatus{ Total: 3, @@ -1756,7 +1756,6 @@ func TestCVO_UpgradePreconditionFailing(t *testing.T) { Image: "image/image:1", URL: configv1.URL("https://example.com/v1.0.1-abc"), }, - Generation: 1, }, SyncWorkerStatus{ Done: 1, @@ -1769,7 +1768,6 @@ func TestCVO_UpgradePreconditionFailing(t *testing.T) { URL: configv1.URL("https://example.com/v1.0.1-abc"), }, LastProgress: time.Unix(1, 0), - Generation: 1, }, SyncWorkerStatus{ Done: 2, @@ -1782,7 +1780,6 @@ func TestCVO_UpgradePreconditionFailing(t *testing.T) { URL: configv1.URL("https://example.com/v1.0.1-abc"), }, LastProgress: time.Unix(2, 0), - Generation: 1, }, SyncWorkerStatus{ Reconciling: true, @@ -1796,7 +1793,6 @@ func TestCVO_UpgradePreconditionFailing(t *testing.T) { URL: configv1.URL("https://example.com/v1.0.1-abc"), }, LastProgress: time.Unix(3, 0), - Generation: 1, }, ) client.ClearActions() @@ -1812,8 +1808,7 @@ func TestCVO_UpgradePreconditionFailing(t *testing.T) { expectUpdateStatus(t, actions[1], "clusterversions", "", &configv1.ClusterVersion{ ObjectMeta: metav1.ObjectMeta{ Name: "version", - ResourceVersion: "1", - Generation: 1, + ResourceVersion: "3", }, Spec: configv1.ClusterVersionSpec{ ClusterID: actual.Spec.ClusterID, @@ -1821,7 +1816,6 @@ func TestCVO_UpgradePreconditionFailing(t *testing.T) { DesiredUpdate: &copied, }, Status: configv1.ClusterVersionStatus{ - ObservedGeneration: 1, Desired: configv1.Release{ Version: "1.0.1-abc", Image: "image/image:1", @@ -1887,7 +1881,7 @@ func TestCVO_UpgradeVerifiedPayload(t *testing.T) { // make the image report unverified payloadErr := &payload.UpdateError{ Reason: "ImageVerificationFailed", - Message: fmt.Sprintf("The update cannot be verified: some random error"), + Message: "The update cannot be verified: some random error", Nested: fmt.Errorf("some random error"), } if !isImageVerificationError(payloadErr) { @@ -1907,7 +1901,7 @@ func TestCVO_UpgradeVerifiedPayload(t *testing.T) { t.Fatal(err) } actions := client.Actions() - if len(actions) != 2 { + if len(actions) < 2 { t.Fatalf("%s", spew.Sdump(actions)) } @@ -1939,7 +1933,7 @@ func TestCVO_UpgradeVerifiedPayload(t *testing.T) { expectUpdateStatus(t, actions[1], "clusterversions", "", &configv1.ClusterVersion{ ObjectMeta: metav1.ObjectMeta{ Name: "version", - ResourceVersion: "1", + ResourceVersion: "2", Generation: 1, }, Spec: configv1.ClusterVersionSpec{ @@ -2063,7 +2057,7 @@ func TestCVO_UpgradeVerifiedPayload(t *testing.T) { expectUpdateStatus(t, actions[1], "clusterversions", "", &configv1.ClusterVersion{ ObjectMeta: metav1.ObjectMeta{ Name: "version", - ResourceVersion: "1", + ResourceVersion: "3", Generation: 2, }, Spec: configv1.ClusterVersionSpec{ @@ -2692,8 +2686,8 @@ func TestCVO_ParallelError(t *testing.T) { expectUpdateStatus(t, actions[1], "clusterversions", "", &configv1.ClusterVersion{ ObjectMeta: metav1.ObjectMeta{ Name: "version", - Generation: 1, - ResourceVersion: "1", + Generation: 0, + ResourceVersion: "2", }, Spec: configv1.ClusterVersionSpec{ ClusterID: clusterUID, @@ -2864,19 +2858,6 @@ func verifyAllStatus(t *testing.T, ch <-chan SyncWorkerStatus, items ...SyncWork } } -func waitFor(t *testing.T, fn func() bool) { - t.Helper() - err := wait.PollImmediate(100*time.Millisecond, 1*time.Second, func() (bool, error) { - return fn(), nil - }) - if err == wait.ErrWaitTimeout { - t.Fatalf("Worker condition was not reached within timeout") - } - if err != nil { - t.Fatal(err) - } -} - // blockingResourceBuilder controls how quickly Apply() is executed and allows error // injection. type blockingResourceBuilder struct { diff --git a/pkg/cvo/cvo_test.go b/pkg/cvo/cvo_test.go index 7e8e31b969..052c42f18c 100644 --- a/pkg/cvo/cvo_test.go +++ b/pkg/cvo/cvo_test.go @@ -17,19 +17,13 @@ import ( "github.com/davecgh/go-spew/spew" "github.com/google/uuid" corev1 "k8s.io/api/core/v1" - apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" - apiextclientv1 "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/diff" - "k8s.io/apimachinery/pkg/watch" - "k8s.io/client-go/discovery" kfake "k8s.io/client-go/kubernetes/fake" - "k8s.io/client-go/rest" ktesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/record" "k8s.io/client-go/util/workqueue" @@ -94,23 +88,6 @@ func (c *clientCVLister) List(selector labels.Selector) (ret []*configv1.Cluster return items, nil } -type proxyLister struct { - Err error - Items []*configv1.Proxy -} - -func (r *proxyLister) List(selector labels.Selector) (ret []*configv1.Proxy, err error) { - return r.Items, r.Err -} -func (r *proxyLister) Get(name string) (*configv1.Proxy, error) { - for _, s := range r.Items { - if s.Name == name { - return s, nil - } - } - return nil, errors.NewNotFound(schema.GroupResource{}, name) -} - type clientCOLister struct { client clientset.Interface } @@ -189,71 +166,6 @@ func (l *cmConfigLister) Get(name string) (*corev1.ConfigMap, error) { return nil, errors.NewNotFound(schema.GroupResource{}, name) } -type crdLister struct { - Err error - Items []*apiextv1beta1.CustomResourceDefinition -} - -func (r *crdLister) Get(name string) (*apiextv1beta1.CustomResourceDefinition, error) { - for _, s := range r.Items { - if s.Name == name { - return s, nil - } - } - return nil, errors.NewNotFound(schema.GroupResource{Resource: "customresourcedefinitions"}, name) -} - -func (r *crdLister) List(selector labels.Selector) (ret []*apiextv1beta1.CustomResourceDefinition, err error) { - return r.Items, r.Err -} - -type fakeApiExtClient struct{} - -func (c *fakeApiExtClient) Discovery() discovery.DiscoveryInterface { - panic("not implemented") -} - -func (c *fakeApiExtClient) ApiextensionsV1beta1() apiextclientv1.ApiextensionsV1beta1Interface { - return c -} - -func (c *fakeApiExtClient) Apiextensions() apiextclientv1.ApiextensionsV1beta1Interface { - return c -} - -func (c *fakeApiExtClient) RESTClient() rest.Interface { panic("not implemented") } - -func (c *fakeApiExtClient) CustomResourceDefinitions() apiextclientv1.CustomResourceDefinitionInterface { - return c -} -func (c *fakeApiExtClient) Create(ctx context.Context, crd *apiextv1beta1.CustomResourceDefinition, createOptions metav1.CreateOptions) (*apiextv1beta1.CustomResourceDefinition, error) { - return crd, nil -} -func (c *fakeApiExtClient) Update(ctx context.Context, crd *apiextv1beta1.CustomResourceDefinition, updateOptions metav1.UpdateOptions) (*apiextv1beta1.CustomResourceDefinition, error) { - panic("not implemented") -} -func (c *fakeApiExtClient) UpdateStatus(ctx context.Context, crd *apiextv1beta1.CustomResourceDefinition, updateOptions metav1.UpdateOptions) (*apiextv1beta1.CustomResourceDefinition, error) { - panic("not implemented") -} -func (c *fakeApiExtClient) Delete(ctx context.Context, name string, options metav1.DeleteOptions) error { - panic("not implemented") -} -func (c *fakeApiExtClient) DeleteCollection(ctx context.Context, options metav1.DeleteOptions, listOptions metav1.ListOptions) error { - panic("not implemented") -} -func (c *fakeApiExtClient) Get(ctx context.Context, name string, options metav1.GetOptions) (*apiextv1beta1.CustomResourceDefinition, error) { - panic("not implemented") -} -func (c *fakeApiExtClient) List(ctx context.Context, opts metav1.ListOptions) (*apiextv1beta1.CustomResourceDefinitionList, error) { - panic("not implemented") -} -func (c *fakeApiExtClient) Watch(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { - panic("not implemented") -} -func (c *fakeApiExtClient) Patch(ctx context.Context, name string, pt types.PatchType, data []byte, patchOptions metav1.PatchOptions, subresources ...string) (result *apiextv1beta1.CustomResourceDefinition, err error) { - panic("not implemented") -} - func TestOperator_sync(t *testing.T) { id := uuid.Must(uuid.NewRandom()).String() @@ -261,7 +173,7 @@ func TestOperator_sync(t *testing.T) { name string key string syncStatus *SyncWorkerStatus - optr Operator + optr *Operator init func(optr *Operator) want bool wantErr func(*testing.T, error) @@ -270,7 +182,7 @@ func TestOperator_sync(t *testing.T) { }{ { name: "create version and status", - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -309,7 +221,7 @@ func TestOperator_sync(t *testing.T) { Message: "unable to apply object", }, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -373,7 +285,7 @@ func TestOperator_sync(t *testing.T) { }, { name: "progressing and previously failed, reconciling", - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -448,7 +360,7 @@ func TestOperator_sync(t *testing.T) { }, { name: "progressing and previously failed, reconciling and multiple completions", - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -524,7 +436,7 @@ func TestOperator_sync(t *testing.T) { }, { name: "progressing and encounters error during image sync", - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -600,7 +512,7 @@ func TestOperator_sync(t *testing.T) { Failure: os.ErrNotExist, Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "4.0.1"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -657,7 +569,7 @@ func TestOperator_sync(t *testing.T) { Failure: os.ErrNotExist, Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "4.0.1"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -703,7 +615,7 @@ func TestOperator_sync(t *testing.T) { Desired: configv1.Release{Image: "image/image:v4.0.1", Version: "4.0.1"}, History: []configv1.UpdateHistory{ // we populate state, but not startedTime - {State: configv1.PartialUpdate, Version: "4.0.1", Image: "image/image:v4.0.1", StartedTime: metav1.Time{time.Unix(0, 0)}}, + {State: configv1.PartialUpdate, Version: "4.0.1", Image: "image/image:v4.0.1", StartedTime: metav1.Time{Time: time.Unix(0, 0)}}, }, VersionHash: "", Conditions: []configv1.ClusterOperatorStatusCondition{ @@ -722,7 +634,7 @@ func TestOperator_sync(t *testing.T) { syncStatus: &SyncWorkerStatus{ Actual: configv1.Release{Image: "image/image:v4.0.1"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Image: "image/image:v4.0.1", }, @@ -782,7 +694,7 @@ func TestOperator_sync(t *testing.T) { syncStatus: &SyncWorkerStatus{ Actual: configv1.Release{Image: "image/image:v4.0.2", Version: "4.0.2"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "4.0.2", Image: "image/image:v4.0.2", @@ -874,7 +786,7 @@ func TestOperator_sync(t *testing.T) { // one). Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "4.0.1"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -980,7 +892,7 @@ func TestOperator_sync(t *testing.T) { Done: 334, Total: 1000, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -1090,7 +1002,7 @@ func TestOperator_sync(t *testing.T) { Actual: configv1.Release{Image: "image/image:v4.0.1"}, Step: "RetrievePayload", }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -1191,7 +1103,7 @@ func TestOperator_sync(t *testing.T) { VersionHash: "xyz", Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "0.0.1-abc"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Image: "image/image:v4.0.1", }, @@ -1268,7 +1180,7 @@ func TestOperator_sync(t *testing.T) { VersionHash: "xyz", Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "0.0.1-abc"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "0.0.1-abc", Image: "image/image:v4.0.1", @@ -1328,7 +1240,7 @@ func TestOperator_sync(t *testing.T) { Completed: 1, Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "0.0.1-abc"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Image: "image/image:v4.0.1", }, @@ -1433,7 +1345,7 @@ func TestOperator_sync(t *testing.T) { Completed: 1, Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "0.0.1-abc"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Image: "image/image:v4.0.1", }, @@ -1511,7 +1423,7 @@ func TestOperator_sync(t *testing.T) { Completed: 1, Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "0.0.1-abc"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Image: "image/image:v4.0.1", }, @@ -1588,7 +1500,7 @@ func TestOperator_sync(t *testing.T) { Completed: 1, Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "0.0.1-abc"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Image: "image/image:v4.0.1", }, @@ -1662,7 +1574,7 @@ func TestOperator_sync(t *testing.T) { Completed: 1, Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "0.0.1-abc"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Image: "image/image:v4.0.1", }, @@ -1747,7 +1659,7 @@ func TestOperator_sync(t *testing.T) { Completed: 1, Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "0.0.1-abc"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Image: "image/image:v4.0.1", }, @@ -1827,7 +1739,7 @@ func TestOperator_sync(t *testing.T) { Completed: 1, Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "4.0.1"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Image: "image/image:v4.0.1", }, @@ -1891,7 +1803,7 @@ func TestOperator_sync(t *testing.T) { Completed: 1, Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "4.0.1"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Image: "image/image:v4.0.1", }, @@ -1966,7 +1878,7 @@ func TestOperator_sync(t *testing.T) { Completed: 1, Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "4.0.1"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Image: "image/image:v4.0.1", }, @@ -2044,7 +1956,7 @@ func TestOperator_sync(t *testing.T) { VersionHash: "y_Kc5IQiIyU=", Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "0.0.1-abc"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "0.0.1-abc", Image: "image/image:v4.0.1", @@ -2106,7 +2018,7 @@ func TestOperator_sync(t *testing.T) { VersionHash: "y_Kc5IQiIyU=", Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "0.0.1-abc"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "0.0.1-abc", Image: "image/image:v4.0.1", @@ -2173,7 +2085,7 @@ func TestOperator_sync(t *testing.T) { Image: "image/image:v4.0.1", Version: "0.0.1-abc", CompletionTime: &defaultCompletionTime, - StartedTime: metav1.Time{time.Unix(0, 0)}, + StartedTime: metav1.Time{Time: time.Unix(0, 0)}, }, }, Desired: configv1.Release{Version: "0.0.1-abc", Image: "image/image:v4.0.1"}, @@ -2197,7 +2109,7 @@ func TestOperator_sync(t *testing.T) { Completed: 1, Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "0.0.1-abc"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Image: "image/image:v4.0.1", }, @@ -2258,7 +2170,7 @@ func TestOperator_sync(t *testing.T) { syncStatus: &SyncWorkerStatus{ Actual: configv1.Release{Image: "image/image:v4.0.1", Version: "0.0.1-abc"}, }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Image: "image/image:v4.0.1", }, @@ -2333,7 +2245,7 @@ func TestOperator_sync(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - optr := &tt.optr + optr := tt.optr if tt.init != nil { tt.init(optr) } @@ -2380,13 +2292,13 @@ func TestOperator_availableUpdatesSync(t *testing.T) { name string key string handler http.HandlerFunc - optr Operator + optr *Operator wantErr func(*testing.T, error) wantUpdates *availableUpdates }{ { name: "when version is missing, do nothing (other loops should create it)", - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -2401,7 +2313,7 @@ func TestOperator_availableUpdatesSync(t *testing.T) { handler: func(w http.ResponseWriter, req *http.Request) { http.Error(w, "bad things", http.StatusInternalServerError) }, - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Image: "image/image:v4.0.1", }, @@ -2440,7 +2352,7 @@ func TestOperator_availableUpdatesSync(t *testing.T) { handler: func(w http.ResponseWriter, req *http.Request) { http.Error(w, "bad things", http.StatusInternalServerError) }, - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", @@ -2481,7 +2393,7 @@ func TestOperator_availableUpdatesSync(t *testing.T) { handler: func(w http.ResponseWriter, req *http.Request) { http.Error(w, "bad things", http.StatusInternalServerError) }, - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Image: "image/image:v4.0.1", @@ -2521,7 +2433,7 @@ func TestOperator_availableUpdatesSync(t *testing.T) { handler: func(w http.ResponseWriter, req *http.Request) { http.Error(w, "bad things", http.StatusInternalServerError) }, - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "4.0.1", @@ -2581,7 +2493,7 @@ func TestOperator_availableUpdatesSync(t *testing.T) { } `) }, - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "4.0.1", @@ -2645,7 +2557,7 @@ func TestOperator_availableUpdatesSync(t *testing.T) { } `) }, - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "4.0.1", @@ -2695,7 +2607,7 @@ func TestOperator_availableUpdatesSync(t *testing.T) { handler: func(w http.ResponseWriter, req *http.Request) { http.Error(w, "bad things", http.StatusInternalServerError) }, - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", minimumUpdateCheckInterval: 1 * time.Minute, availableUpdates: &availableUpdates{ @@ -2798,13 +2710,13 @@ func TestOperator_upgradeableSync(t *testing.T) { tests := []struct { name string key string - optr Operator + optr *Operator wantErr func(*testing.T, error) want *upgradeable }{ { name: "when version is missing, do nothing (other loops should create it)", - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -2816,7 +2728,7 @@ func TestOperator_upgradeableSync(t *testing.T) { }, { name: "report error condition when overrides is set for version", - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Image: "image/image:v4.0.1", }, @@ -2853,7 +2765,7 @@ func TestOperator_upgradeableSync(t *testing.T) { }, { name: "report error condition when the single clusteroperator is not upgradeable", - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", @@ -2902,7 +2814,7 @@ func TestOperator_upgradeableSync(t *testing.T) { }, { name: "report error condition when single clusteroperator is not upgradeable and another has no conditions", - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", @@ -2959,7 +2871,7 @@ func TestOperator_upgradeableSync(t *testing.T) { }, { name: "report error condition when single clusteroperator is not upgradeable and another is upgradeable", - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", @@ -3019,7 +2931,7 @@ func TestOperator_upgradeableSync(t *testing.T) { }, { name: "report error condition when two clusteroperators are not upgradeable", - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", @@ -3081,7 +2993,7 @@ func TestOperator_upgradeableSync(t *testing.T) { }, { name: "report error condition when clusteroperators and version are not upgradeable", - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", @@ -3156,7 +3068,7 @@ func TestOperator_upgradeableSync(t *testing.T) { }, { name: "no error conditions", - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", @@ -3186,7 +3098,7 @@ func TestOperator_upgradeableSync(t *testing.T) { }, { name: "no error conditions", - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", @@ -3218,7 +3130,7 @@ func TestOperator_upgradeableSync(t *testing.T) { }, { name: "no error conditions", - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", @@ -3317,11 +3229,6 @@ func expectCreate(t *testing.T, a ktesting.Action, resource, namespace string, o expectMutation(t, a, "create", resource, "", namespace, obj) } -func expectUpdate(t *testing.T, a ktesting.Action, resource, namespace string, obj interface{}) { - t.Helper() - expectMutation(t, a, "update", resource, "", namespace, obj) -} - func expectUpdateStatus(t *testing.T, a ktesting.Action, resource, namespace string, obj interface{}) { t.Helper() expectMutation(t, a, "update", resource, "status", namespace, obj) diff --git a/pkg/cvo/egress.go b/pkg/cvo/egress.go index fa5260fd87..eabadf8765 100644 --- a/pkg/cvo/egress.go +++ b/pkg/cvo/egress.go @@ -21,14 +21,12 @@ func (optr *Operator) getHTTPSProxyURL() (*url.URL, error) { return nil, err } - if &proxy.Spec != nil { - if proxy.Spec.HTTPSProxy != "" { - proxyURL, err := url.Parse(proxy.Spec.HTTPSProxy) - if err != nil { - return nil, err - } - return proxyURL, nil + if proxy.Spec.HTTPSProxy != "" { + proxyURL, err := url.Parse(proxy.Spec.HTTPSProxy) + if err != nil { + return nil, err } + return proxyURL, nil } return nil, nil } diff --git a/pkg/cvo/internal/generic.go b/pkg/cvo/internal/generic.go index bcf95b6ec7..b04b371670 100644 --- a/pkg/cvo/internal/generic.go +++ b/pkg/cvo/internal/generic.go @@ -2,7 +2,6 @@ package internal import ( "context" - "encoding/json" "fmt" "github.com/openshift/cluster-version-operator/lib/resourceapply" @@ -10,10 +9,8 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/client-go/dynamic" "k8s.io/klog/v2" @@ -120,15 +117,3 @@ func (b *genericBuilder) Do(ctx context.Context) error { _, _, err := applyUnstructured(ctx, b.client, ud) return err } - -func createPatch(original, modified runtime.Object) ([]byte, error) { - originalData, err := json.Marshal(original) - if err != nil { - return nil, err - } - modifiedData, err := json.Marshal(modified) - if err != nil { - return nil, err - } - return strategicpatch.CreateTwoWayMergePatch(originalData, modifiedData, original) -} diff --git a/pkg/cvo/internal/operatorstatus.go b/pkg/cvo/internal/operatorstatus.go index 5c76a5eacb..cf7cb8057d 100644 --- a/pkg/cvo/internal/operatorstatus.go +++ b/pkg/cvo/internal/operatorstatus.go @@ -5,7 +5,6 @@ import ( "fmt" "sort" "strings" - "unicode" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -232,10 +231,3 @@ func checkOperatorHealth(ctx context.Context, client ClusterOperatorsGetter, exp return nil } - -func lowerFirst(str string) string { - for i, v := range str { - return string(unicode.ToLower(v)) + str[i+1:] - } - return "" -} diff --git a/pkg/cvo/internal/operatorstatus_test.go b/pkg/cvo/internal/operatorstatus_test.go index 5608e1aaee..8877a2ecd0 100644 --- a/pkg/cvo/internal/operatorstatus_test.go +++ b/pkg/cvo/internal/operatorstatus_test.go @@ -43,7 +43,7 @@ func Test_checkOperatorHealth(t *testing.T) { }, }, expErr: &payload.UpdateError{ - Nested: apierrors.NewNotFound(schema.GroupResource{"", "clusteroperator"}, "test-co"), + Nested: apierrors.NewNotFound(schema.GroupResource{Resource: "clusteroperator"}, "test-co"), UpdateEffect: payload.UpdateEffectNone, Reason: "ClusterOperatorNotAvailable", Message: "Cluster operator test-co has not yet reported success", diff --git a/pkg/cvo/status_test.go b/pkg/cvo/status_test.go index bea36a054c..e0b57bd915 100644 --- a/pkg/cvo/status_test.go +++ b/pkg/cvo/status_test.go @@ -15,8 +15,6 @@ import ( ) func Test_mergeEqualVersions(t *testing.T) { - type args struct { - } tests := []struct { name string current *configv1.UpdateHistory @@ -144,11 +142,9 @@ func Test_pruneStatusHistory(t *testing.T) { func TestOperator_syncFailingStatus(t *testing.T) { ctx := context.Background() - type args struct { - } tests := []struct { name string - optr Operator + optr *Operator init func(optr *Operator) wantErr func(*testing.T, error) wantActions func(*testing.T, *Operator) @@ -159,7 +155,7 @@ func TestOperator_syncFailingStatus(t *testing.T) { }{ { ierr: fmt.Errorf("bad"), - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -238,7 +234,7 @@ func TestOperator_syncFailingStatus(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - optr := &tt.optr + optr := tt.optr if tt.init != nil { tt.init(optr) } diff --git a/pkg/cvo/sync_test.go b/pkg/cvo/sync_test.go index bf4a127d35..aa6a2c4900 100644 --- a/pkg/cvo/sync_test.go +++ b/pkg/cvo/sync_test.go @@ -67,10 +67,10 @@ func Test_SyncWorker_apply(t *testing.T) { t.Fatalf("unexpected %d actions", len(actions)) } - if got, exp := actions[0], (newAction(schema.GroupVersionKind{"test.cvo.io", "v1", "TestA"}, "default", "testa")); !reflect.DeepEqual(got, exp) { + if got, exp := actions[0], (newAction(schema.GroupVersionKind{Group: "test.cvo.io", Version: "v1", Kind: "TestA"}, "default", "testa")); !reflect.DeepEqual(got, exp) { t.Fatalf("%s", diff.ObjectReflectDiff(exp, got)) } - if got, exp := actions[1], (newAction(schema.GroupVersionKind{"test.cvo.io", "v1", "TestB"}, "default", "testb")); !reflect.DeepEqual(got, exp) { + if got, exp := actions[1], (newAction(schema.GroupVersionKind{Group: "test.cvo.io", Version: "v1", Kind: "TestB"}, "default", "testb")); !reflect.DeepEqual(got, exp) { t.Fatalf("%s", diff.ObjectReflectDiff(exp, got)) } }, @@ -94,7 +94,7 @@ func Test_SyncWorker_apply(t *testing.T) { }`, }, reactors: map[action]error{ - newAction(schema.GroupVersionKind{"test.cvo.io", "v1", "TestA"}, "default", "testa"): &meta.NoResourceMatchError{}, + newAction(schema.GroupVersionKind{Group: "test.cvo.io", Version: "v1", Kind: "TestA"}, "default", "testa"): &meta.NoResourceMatchError{}, }, cancelAfter: 2, wantErr: true, @@ -104,7 +104,7 @@ func Test_SyncWorker_apply(t *testing.T) { t.Fatalf("unexpected %d actions", len(actions)) } - if got, exp := actions[0], (newAction(schema.GroupVersionKind{"test.cvo.io", "v1", "TestA"}, "default", "testa")); !reflect.DeepEqual(got, exp) { + if got, exp := actions[0], (newAction(schema.GroupVersionKind{Group: "test.cvo.io", Version: "v1", Kind: "TestA"}, "default", "testa")); !reflect.DeepEqual(got, exp) { t.Fatalf("%s", diff.ObjectReflectDiff(exp, got)) } }, @@ -129,8 +129,8 @@ func Test_SyncWorker_apply(t *testing.T) { } r := &recorder{} testMapper := resourcebuilder.NewResourceMapper() - testMapper.RegisterGVK(schema.GroupVersionKind{"test.cvo.io", "v1", "TestA"}, newTestBuilder(r, test.reactors)) - testMapper.RegisterGVK(schema.GroupVersionKind{"test.cvo.io", "v1", "TestB"}, newTestBuilder(r, test.reactors)) + testMapper.RegisterGVK(schema.GroupVersionKind{Group: "test.cvo.io", Version: "v1", Kind: "TestA"}, newTestBuilder(r, test.reactors)) + testMapper.RegisterGVK(schema.GroupVersionKind{Group: "test.cvo.io", Version: "v1", Kind: "TestB"}, newTestBuilder(r, test.reactors)) testMapper.AddToMap(resourcebuilder.Mapper) worker := &SyncWorker{eventRecorder: record.NewFakeRecorder(100)} @@ -144,7 +144,7 @@ func Test_SyncWorker_apply(t *testing.T) { remainingErrors: test.cancelAfter, } - worker.apply(ctx, up, &SyncWork{}, 1, &statusWrapper{w: worker, previousStatus: worker.Status()}) + _ = worker.apply(ctx, up, &SyncWork{}, 1, &statusWrapper{w: worker, previousStatus: worker.Status()}) test.check(t, r.actions) }) } @@ -410,16 +410,6 @@ func (r *fakeSyncRecorder) Update(generation int64, desired configv1.Update, ove return r.Returns } -type fakeResourceBuilder struct { - M []*manifest.Manifest - Err error -} - -func (b *fakeResourceBuilder) Apply(m *manifest.Manifest) error { - b.M = append(b.M, m) - return b.Err -} - type fakeDirectoryRetriever struct { lock sync.Mutex diff --git a/pkg/cvo/sync_worker.go b/pkg/cvo/sync_worker.go index 6c2d98fe0e..ca634ecb20 100644 --- a/pkg/cvo/sync_worker.go +++ b/pkg/cvo/sync_worker.go @@ -14,7 +14,6 @@ import ( "github.com/prometheus/client_golang/prometheus" "golang.org/x/time/rate" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/errors" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" @@ -23,7 +22,6 @@ import ( configv1 "github.com/openshift/api/config/v1" - "github.com/openshift/cluster-version-operator/lib/resourcebuilder" "github.com/openshift/cluster-version-operator/pkg/payload" "github.com/openshift/cluster-version-operator/pkg/payload/precondition" "github.com/openshift/library-go/pkg/manifest" @@ -1000,10 +998,6 @@ func newClusterOperatorsNotAvailable(errs []error) error { return nil } - nested := make([]error, 0, len(errs)) - for _, err := range errs { - nested = append(nested, err) - } sort.Strings(names) name := strings.Join(names, ", ") return &payload.UpdateError{ @@ -1073,16 +1067,6 @@ func getOverrideForManifest(overrides []configv1.ComponentOverride, manifest *ma return configv1.ComponentOverride{}, false } -// ownerKind contains the schema.GroupVersionKind for type that owns objects managed by CVO. -var ownerKind = configv1.SchemeGroupVersion.WithKind("ClusterVersion") - -func ownerRefModifier(config *configv1.ClusterVersion) resourcebuilder.MetaV1ObjectModifierFunc { - oref := metav1.NewControllerRef(config, ownerKind) - return func(obj metav1.Object) { - obj.SetOwnerReferences([]metav1.OwnerReference{*oref}) - } -} - // runThrottledStatusNotifier invokes fn every time ch is updated, but no more often than once // every interval. If bucket is non-zero then the channel is throttled like a rate limiter bucket. func runThrottledStatusNotifier(ctx context.Context, interval time.Duration, bucket int, ch <-chan SyncWorkerStatus, fn func()) { diff --git a/pkg/cvo/sync_worker_test.go b/pkg/cvo/sync_worker_test.go index 507948c783..ddce260d0e 100644 --- a/pkg/cvo/sync_worker_test.go +++ b/pkg/cvo/sync_worker_test.go @@ -85,25 +85,21 @@ func Test_statusWrapper_ReportProgress(t *testing.T) { w.Report(tt.next) close(w.w.report) if tt.want { - select { - case evt, ok := <-w.w.report: - if !ok { - t.Fatalf("no event") - } - if tt.wantProgress != (!evt.LastProgress.IsZero()) { - t.Errorf("unexpected progress timestamp: %#v", evt) - } - evt.LastProgress = time.Time{} - if !reflect.DeepEqual(evt, tt.next) { - t.Fatalf("unexpected: %#v", evt) - } + evt, ok := <-w.w.report + if !ok { + t.Fatalf("no event") + } + if tt.wantProgress != (!evt.LastProgress.IsZero()) { + t.Errorf("unexpected progress timestamp: %#v", evt) + } + evt.LastProgress = time.Time{} + if !reflect.DeepEqual(evt, tt.next) { + t.Fatalf("unexpected: %#v", evt) } } else { - select { - case evt, ok := <-w.w.report: - if ok { - t.Fatalf("unexpected event: %#v", evt) - } + evt, ok := <-w.w.report + if ok { + t.Fatalf("unexpected event: %#v", evt) } } }) @@ -138,11 +134,9 @@ func Test_statusWrapper_ReportGeneration(t *testing.T) { w.Report(tt.next) close(w.w.report) - select { - case evt := <-w.w.report: - if tt.want != evt.Generation { - t.Fatalf("mismatch: expected generation: %d, got generation: %d", tt.want, evt.Generation) - } + evt := <-w.w.report + if tt.want != evt.Generation { + t.Fatalf("mismatch: expected generation: %d, got generation: %d", tt.want, evt.Generation) } }) } diff --git a/pkg/payload/precondition/clusterversion/upgradable_test.go b/pkg/payload/precondition/clusterversion/upgradable_test.go index 3fe8bd47f4..30d4d9d456 100644 --- a/pkg/payload/precondition/clusterversion/upgradable_test.go +++ b/pkg/payload/precondition/clusterversion/upgradable_test.go @@ -125,7 +125,7 @@ func TestUpgradeableRun(t *testing.T) { Message: fmt.Sprintf("set to %v", *tc.upgradeable), }) } - cvLister := fakeClusterVersionLister(clusterVersion) + cvLister := fakeClusterVersionLister(t, clusterVersion) instance := NewUpgradeable(cvLister) err := instance.Run(context.TODO(), precondition.ReleaseContext{DesiredVersion: tc.desiredVersion}, clusterVersion) @@ -144,12 +144,15 @@ func TestUpgradeableRun(t *testing.T) { } } -func fakeClusterVersionLister(clusterVersion *configv1.ClusterVersion) configv1listers.ClusterVersionLister { +func fakeClusterVersionLister(t *testing.T, clusterVersion *configv1.ClusterVersion) configv1listers.ClusterVersionLister { indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) if clusterVersion == nil { return configv1listers.NewClusterVersionLister(indexer) } - indexer.Add(clusterVersion) + err := indexer.Add(clusterVersion) + if err != nil { + t.Fatal(err) + } return configv1listers.NewClusterVersionLister(indexer) } diff --git a/pkg/payload/precondition/precondition_test.go b/pkg/payload/precondition/precondition_test.go index 2a6649977a..137f37d952 100644 --- a/pkg/payload/precondition/precondition_test.go +++ b/pkg/payload/precondition/precondition_test.go @@ -20,7 +20,7 @@ func TestSummarize(t *testing.T) { input: []error{&Error{ Nested: nil, Reason: "NotAllowedFeatureGateSet", - Message: fmt.Sprintf("Feature Gate random is set for the cluster. This Feature Gate turns on features that are not part of the normal supported platform."), + Message: "Feature Gate random is set for the cluster. This Feature Gate turns on features that are not part of the normal supported platform.", Name: "FeatureGate", }}, exp: `Precondition "FeatureGate" failed because of "NotAllowedFeatureGateSet": Feature Gate random is set for the cluster. This Feature Gate turns on features that are not part of the normal supported platform.`, @@ -33,12 +33,12 @@ func TestSummarize(t *testing.T) { input: []error{&Error{ Nested: nil, Reason: "NotAllowedFeatureGateSet", - Message: fmt.Sprintf("Feature Gate random is set for the cluster. This Feature Gate turns on features that are not part of the normal supported platform."), + Message: "Feature Gate random is set for the cluster. This Feature Gate turns on features that are not part of the normal supported platform.", Name: "FeatureGate", }, &Error{ Nested: nil, Reason: "NotAllowedFeatureGateSet", - Message: fmt.Sprintf("Feature Gate random-2 is set for the cluster. This Feature Gate turns on features that are not part of the normal supported platform."), + Message: "Feature Gate random-2 is set for the cluster. This Feature Gate turns on features that are not part of the normal supported platform.", Name: "FeatureGate", }}, exp: `Multiple precondition checks failed: @@ -50,7 +50,7 @@ func TestSummarize(t *testing.T) { &Error{ Nested: nil, Reason: "NotAllowedFeatureGateSet", - Message: fmt.Sprintf("Feature Gate random is set for the cluster. This Feature Gate turns on features that are not part of the normal supported platform."), + Message: "Feature Gate random is set for the cluster. This Feature Gate turns on features that are not part of the normal supported platform.", Name: "FeatureGate", }}, exp: `Multiple precondition checks failed: diff --git a/pkg/payload/task_graph.go b/pkg/payload/task_graph.go index 4ae4577484..d31c0847eb 100644 --- a/pkg/payload/task_graph.go +++ b/pkg/payload/task_graph.go @@ -386,15 +386,6 @@ func (g *TaskGraph) Tree() string { return strings.Join(out, "\n") } -func covers(all []int, some []int) bool { - for _, i := range some { - if all[i] == 0 { - return false - } - } - return true -} - func (g *TaskGraph) bulkAdd(nodes []*TaskNode, inNodes []int) []int { from := len(g.Nodes) g.Nodes = append(g.Nodes, nodes...) diff --git a/pkg/start/start_integration_test.go b/pkg/start/start_integration_test.go index 534a3c6549..3278d5f124 100644 --- a/pkg/start/start_integration_test.go +++ b/pkg/start/start_integration_test.go @@ -41,8 +41,8 @@ import ( func init() { klog.InitFlags(flag.CommandLine) - flag.CommandLine.Lookup("v").Value.Set("5") - flag.CommandLine.Lookup("alsologtostderr").Value.Set("true") + _ = flag.CommandLine.Lookup("v").Value.Set("5") + _ = flag.CommandLine.Lookup("alsologtostderr").Value.Set("true") } var ( @@ -685,7 +685,7 @@ func TestIntegrationCVO_cincinnatiRequest(t *testing.T) { id, _ := uuid.NewRandom() - client.ConfigV1().ClusterVersions().Create( + _, err = client.ConfigV1().ClusterVersions().Create( ctx, &configv1.ClusterVersion{ ObjectMeta: metav1.ObjectMeta{ @@ -699,6 +699,9 @@ func TestIntegrationCVO_cincinnatiRequest(t *testing.T) { }, metav1.CreateOptions{}, ) + if err != nil { + t.Fatal(err) + } dir, err := ioutil.TempDir("", "cvo-test") if err != nil {