From ef12c867e7598bffe8ff8591b421765c20a260ff Mon Sep 17 00:00:00 2001 From: bharathi-tenneti Date: Wed, 8 Apr 2020 15:43:36 -0400 Subject: [PATCH 1/4] Check for empty gvk --- pkg/helm/controller/controller.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/helm/controller/controller.go b/pkg/helm/controller/controller.go index da684d3276..de50d93882 100644 --- a/pkg/helm/controller/controller.go +++ b/pkg/helm/controller/controller.go @@ -118,6 +118,9 @@ func watchDependentResources(mgr manager.Manager, r *HelmOperatorReconciler, c c } gvk := u.GroupVersionKind() + if gvk == nil { + continue + } m.RLock() _, ok := watches[gvk] m.RUnlock() From 2d94d2371f6a29748338438beae0be00fa30a738 Mon Sep 17 00:00:00 2001 From: bharathi-tenneti Date: Wed, 8 Apr 2020 15:54:22 -0400 Subject: [PATCH 2/4] Check for empty gvk --- cmd/operator-sdk/add/crd.go | 2 +- pkg/helm/controller/controller.go | 2 +- pkg/helm/watches/watches_test.go | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/operator-sdk/add/crd.go b/cmd/operator-sdk/add/crd.go index e472c85a15..f5de001118 100644 --- a/cmd/operator-sdk/add/crd.go +++ b/cmd/operator-sdk/add/crd.go @@ -105,7 +105,7 @@ func crdFunc(cmd *cobra.Command, args []string) error { gcfg := gen.Config{} crd := gencrd.NewCRDNonGo(gcfg, *resource, crdVersion) if err := crd.Generate(); err != nil { - log.Fatalf("Error generating CRD for %s: %w", resource, err) + log.Fatalf("Error generating CRD for %s: %v", resource, err) } // update deploy/role.yaml for the given resource r. diff --git a/pkg/helm/controller/controller.go b/pkg/helm/controller/controller.go index de50d93882..7587e05cea 100644 --- a/pkg/helm/controller/controller.go +++ b/pkg/helm/controller/controller.go @@ -118,7 +118,7 @@ func watchDependentResources(mgr manager.Manager, r *HelmOperatorReconciler, c c } gvk := u.GroupVersionKind() - if gvk == nil { + if gvk.Empty() { continue } m.RLock() diff --git a/pkg/helm/watches/watches_test.go b/pkg/helm/watches/watches_test.go index f5f98fb25a..61786de62e 100644 --- a/pkg/helm/watches/watches_test.go +++ b/pkg/helm/watches/watches_test.go @@ -152,19 +152,19 @@ foo: bar t.Run(tc.name, func(t *testing.T) { tmp, err := ioutil.TempFile("", "watches.yaml") if err != nil { - t.Fatalf("Failed to create temporary watches.yaml file: %w", err) + t.Fatalf("Failed to create temporary watches.yaml file: %v", err) } defer func() { _ = os.Remove(tmp.Name()) }() if _, err := tmp.WriteString(tc.data); err != nil { - t.Fatalf("Failed to write data to temporary watches.yaml file: %w", err) + t.Fatalf("Failed to write data to temporary watches.yaml file: %v", err) } if err := tmp.Close(); err != nil { - t.Fatalf("Failed to close temporary watches.yaml file: %w", err) + t.Fatalf("Failed to close temporary watches.yaml file: %v", err) } for k, v := range tc.env { if err := os.Setenv(k, v); err != nil { - t.Fatalf("Failed to set environment variable %q: %w", k, err) + t.Fatalf("Failed to set environment variable %q: %v", k, err) } } @@ -172,7 +172,7 @@ foo: bar for k := range tc.env { if err := os.Unsetenv(k); err != nil { - t.Fatalf("Failed to unset environment variable %q: %w", k, err) + t.Fatalf("Failed to unset environment variable %q: %v", k, err) } } }) @@ -182,7 +182,7 @@ foo: bar func doTest(t *testing.T, tc testCase, watchesFile string) { watches, err := Load(watchesFile) if !tc.expectErr && err != nil { - t.Fatalf("Expected no error; got error: %w", err) + t.Fatalf("Expected no error; got error: %v", err) } else if tc.expectErr && err == nil { t.Fatalf("Expected error; got no error") } From 54b94edeec3dd11dd80dea3fba01fa4c92f5a28c Mon Sep 17 00:00:00 2001 From: bharathi-tenneti Date: Mon, 13 Apr 2020 15:45:11 -0400 Subject: [PATCH 3/4] Reverted to %w --- cmd/operator-sdk/add/crd.go | 2 +- pkg/helm/watches/watches_test.go | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/operator-sdk/add/crd.go b/cmd/operator-sdk/add/crd.go index f5de001118..e472c85a15 100644 --- a/cmd/operator-sdk/add/crd.go +++ b/cmd/operator-sdk/add/crd.go @@ -105,7 +105,7 @@ func crdFunc(cmd *cobra.Command, args []string) error { gcfg := gen.Config{} crd := gencrd.NewCRDNonGo(gcfg, *resource, crdVersion) if err := crd.Generate(); err != nil { - log.Fatalf("Error generating CRD for %s: %v", resource, err) + log.Fatalf("Error generating CRD for %s: %w", resource, err) } // update deploy/role.yaml for the given resource r. diff --git a/pkg/helm/watches/watches_test.go b/pkg/helm/watches/watches_test.go index 61786de62e..f5f98fb25a 100644 --- a/pkg/helm/watches/watches_test.go +++ b/pkg/helm/watches/watches_test.go @@ -152,19 +152,19 @@ foo: bar t.Run(tc.name, func(t *testing.T) { tmp, err := ioutil.TempFile("", "watches.yaml") if err != nil { - t.Fatalf("Failed to create temporary watches.yaml file: %v", err) + t.Fatalf("Failed to create temporary watches.yaml file: %w", err) } defer func() { _ = os.Remove(tmp.Name()) }() if _, err := tmp.WriteString(tc.data); err != nil { - t.Fatalf("Failed to write data to temporary watches.yaml file: %v", err) + t.Fatalf("Failed to write data to temporary watches.yaml file: %w", err) } if err := tmp.Close(); err != nil { - t.Fatalf("Failed to close temporary watches.yaml file: %v", err) + t.Fatalf("Failed to close temporary watches.yaml file: %w", err) } for k, v := range tc.env { if err := os.Setenv(k, v); err != nil { - t.Fatalf("Failed to set environment variable %q: %v", k, err) + t.Fatalf("Failed to set environment variable %q: %w", k, err) } } @@ -172,7 +172,7 @@ foo: bar for k := range tc.env { if err := os.Unsetenv(k); err != nil { - t.Fatalf("Failed to unset environment variable %q: %v", k, err) + t.Fatalf("Failed to unset environment variable %q: %w", k, err) } } }) @@ -182,7 +182,7 @@ foo: bar func doTest(t *testing.T, tc testCase, watchesFile string) { watches, err := Load(watchesFile) if !tc.expectErr && err != nil { - t.Fatalf("Expected no error; got error: %v", err) + t.Fatalf("Expected no error; got error: %w", err) } else if tc.expectErr && err == nil { t.Fatalf("Expected error; got no error") } From 66064455566557a3c61ca53437e4a79eab414aff Mon Sep 17 00:00:00 2001 From: bharathi-tenneti Date: Mon, 13 Apr 2020 17:16:18 -0400 Subject: [PATCH 4/4] Added CHANGELOG entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 019248e789..89e56730df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,8 @@ - `operator-sdk generate csv` and `operator-sdk test local` now parse multi-manifest files correctly. ([#2758](https://github.com/operator-framework/operator-sdk/pull/2758)) - Fixed CRD validation generation issue with `status.Conditions`. ([#2739](https://github.com/operator-framework/operator-sdk/pull/2739)) - Fix issue faced in the reconciliation when arrays are used in the config YAML files for Helm based-operators. ([#2777](https://github.com/operator-framework/operator-sdk/pull/2777)) +- Fixed issue in helm-operator where empty resource in release manifest caused failures while setting up watches for dependent resources. ([#2831](https://github.com/operator-framework/operator-sdk/pull/2831)) + ## v0.16.0