From 30a8a7776326acb46f06c8c85775b4ea608a35b2 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Wed, 27 Nov 2024 13:19:05 -0500 Subject: [PATCH 1/5] Adjust golangci-lint linters Added new linters (some commented out if not needed) and removed deprecated linters. Related to https://github.com/submariner-io/enhancements/issues/231 Signed-off-by: Tom Pantelis --- .golangci.yml | 43 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 9cc46fddb..5d1ae980a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -53,29 +53,38 @@ linters-settings: linters: disable-all: true enable: + - asasalint - asciicheck - bidichk - bodyclose + # - canonicalheader # This is a slow linter and we don't use the net/http.Header API + - containedctx - contextcheck - copyloopvar # - cyclop # This is equivalent to gocyclo + - decorder # - depguard # depguard now denies by default, it should only be enabled if we actually use it - dogsled - dupl - durationcheck - errcheck + - errchkjson - errorlint - errname - exhaustive - # - exhaustivestruct # Not recommended for general use - meant to be used only for special cases + # - exhaustruct # This is too cumbersome as it requires all string, int, pointer et al fields to be initialized even when the + # type's default suffices, which is most of the time + - fatcontext # - forbidigo # We don't forbid any statements # - forcetypeassert # There are many unchecked type assertions that would be the result of a programming error so the # reasonable recourse would be to panic anyway if checked so this doesn't seem useful # - funlen # gocyclo is enabled which is generally a better metric than simply LOC. - gci - ginkgolinter + - gocheckcompilerdirectives # - gochecknoglobals # We don't want to forbid global variable constants # - gochecknoinits # We use init functions for valid reasons + # - gochecksumtype # The usefulness is very narrow - gocognit - goconst - gocritic @@ -87,24 +96,30 @@ linters: - gofumpt - goheader - goimports - # - golint # Deprecated since v1.41.0 - # - gomnd # It doesn't seem useful in general to enforce constants for all numeric values # - gomoddirectives # We don't want to forbid the 'replace' directive # - gomodguard # We don't block any modules # - goprintffuncname # This doesn't seem useful at all + # - gosmopolitan # This is related to internationalization which is not a concern for us - gosec - gosimple - govet - # - ifshort # This is a style preference and doesn't seem compelling + - grouper + - iface - importas + - inamedparam - ineffassign + # - interfacebloat # We track complexity elsewhere + - intrange # - ireturn # The argument to always "Return Concrete Types" doesn't seem compelling. It is perfectly valid to return - # an interface to avoid exposing the entire underlying struct - # - interfacer # Deprecated since v1.38.0 + # an interface to avoid exposing the entire underlying struct - lll + - loggercheck + - maintidx - makezero - # - maligned # Deprecated since v1.38.0 + - mirror - misspell + # - mnd # It doesn't seem useful in general to enforce constants for all numeric values + - musttag - nakedret # - nestif # This calculates cognitive complexity but we're doing that elsewhere - nilerr @@ -112,18 +127,28 @@ linters: # - nlreturn # This is reasonable with a block-size of 2 but setting it above isn't honored # - noctx # We don't send HTTP requests - nolintlint + - nonamedreturns + # - nosprintfhostport # The use of this is very narrow # - paralleltest # Not relevant for Ginkgo UTs + - perfsprint - prealloc - predeclared - promlinter + # - protogetter # We don't use protobuf + - reassign + - recvcheck - revive # - rowserrcheck # We don't use SQL - # - scopelint # Deprecated since v1.39.0 + # - sloglint # We don't use log/slog + # - spancheck # We don't use OpenTelemetry/OpenCensus # - sqlclosecheck # We don't use SQL - staticcheck - stylecheck + - tagalign # - tagliatelle # Inconsistent with stylecheck and not as good # - tenv # Not relevant for our Ginkgo UTs + # - testableexamples # We don't need this + # - testifylint # We don't use testify - testpackage # - thelper # Not relevant for our Ginkgo UTs # - tparallel # Not relevant for our Ginkgo UTs @@ -131,11 +156,13 @@ linters: - unconvert - unparam - unused + - usestdlibvars # - varnamelen # It doesn't seem necessary to enforce a minimum variable name length - wastedassign - whitespace - wrapcheck - wsl + # - zerologlint # We use zerolog indirectly so this isn't needed issues: exclude-rules: # Allow dot-imports for Gomega BDD directives per idiomatic Gomega From 97e7bd4bcb25f36b39ebcbd982c849dcc1de5912 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Wed, 27 Nov 2024 12:10:51 -0500 Subject: [PATCH 2/5] Address errchkjson linter violations "Error return value of `encoding/json.MarshalIndent` is not checked" Signed-off-by: Tom Pantelis --- test/e2e/cleanup/submariner.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/e2e/cleanup/submariner.go b/test/e2e/cleanup/submariner.go index 53644e6c6..259d37eb2 100644 --- a/test/e2e/cleanup/submariner.go +++ b/test/e2e/cleanup/submariner.go @@ -21,7 +21,6 @@ package cleanup import ( "bytes" "context" - "encoding/json" "fmt" "io" "sync" @@ -29,6 +28,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/submariner-io/admiral/pkg/names" + "github.com/submariner-io/admiral/pkg/resource" "github.com/submariner-io/admiral/pkg/watcher" "github.com/submariner-io/shipyard/test/e2e/framework" operatorv1alpha1 "github.com/submariner-io/submariner-operator/api/v1alpha1" @@ -311,14 +311,14 @@ func (m *podMonitor) assertUninstallPodsCompleted() { continue } - status, _ := json.MarshalIndent(info.status, "", " ") + status := resource.ToJSON(info.status) log := info.prevLog if log == "" { log = info.log } - Fail(fmt.Sprintf("Pod %q did not complete\nSTATUS: %s\n\nLOG\n: %s\n", name, string(status), log)) + Fail(fmt.Sprintf("Pod %q did not complete\nSTATUS: %s\n\nLOG\n: %s\n", name, status, log)) } } From 74821c0496208483222c0cc2bfad5ba4beac4590 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Wed, 27 Nov 2024 12:18:09 -0500 Subject: [PATCH 3/5] Address inamedparam linter violations "interface method <...> must have named param for type <...>" Signed-off-by: Tom Pantelis --- pkg/crd/updater.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/crd/updater.go b/pkg/crd/updater.go index 96873debd..144d5226f 100644 --- a/pkg/crd/updater.go +++ b/pkg/crd/updater.go @@ -34,15 +34,17 @@ import ( ) type baseUpdater interface { - Create(context.Context, *apiextensions.CustomResourceDefinition, metav1.CreateOptions) (*apiextensions.CustomResourceDefinition, error) - Update(context.Context, *apiextensions.CustomResourceDefinition, metav1.UpdateOptions) (*apiextensions.CustomResourceDefinition, error) - Get(context.Context, string, metav1.GetOptions) (*apiextensions.CustomResourceDefinition, error) - Delete(context.Context, string, metav1.DeleteOptions) error + Create(ctx context.Context, crd *apiextensions.CustomResourceDefinition, options metav1.CreateOptions, + ) (*apiextensions.CustomResourceDefinition, error) + Update(ctx context.Context, crd *apiextensions.CustomResourceDefinition, options metav1.UpdateOptions, + ) (*apiextensions.CustomResourceDefinition, error) + Get(ctx context.Context, name string, options metav1.GetOptions) (*apiextensions.CustomResourceDefinition, error) + Delete(ctx context.Context, name string, options metav1.DeleteOptions) error } type Updater interface { baseUpdater - CreateOrUpdateFromEmbedded(context.Context, string) (bool, error) + CreateOrUpdateFromEmbedded(ctx context.Context, name string) (bool, error) } type updater struct { From b1c7307885cd3da0f9a8701ae7f3f82ee0e90769 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Wed, 27 Nov 2024 12:31:43 -0500 Subject: [PATCH 4/5] Address perfsprint linter violations - "fmt.Errorf can be replaced with errors.New" - "fmt.Sprintf can be replaced with string concatenation" - "fmt.Sprint can be replaced with faster strconv.FormatUint" - "fmt.Sprintf can be replaced with faster strconv.Itoa" Signed-off-by: Tom Pantelis --- controllers/metrics/metrics.go | 3 +-- controllers/submariner/gateway_resources.go | 3 +-- controllers/submariner/loadbalancer_resources.go | 4 ++-- controllers/submariner/metrics_proxy_resources.go | 7 ++++--- controllers/submariner/submariner_controller_test.go | 3 +-- controllers/submariner/submariner_networkdiscovery.go | 5 ++--- main.go | 2 +- pkg/discovery/clustersetip/clustersetip.go | 3 +-- pkg/discovery/clustersetip/config_map.go | 3 +-- pkg/discovery/globalnet/config_map.go | 4 ++-- pkg/discovery/globalnet/globalnet.go | 3 +-- pkg/discovery/network/generic.go | 2 +- pkg/embeddedyamls/generators/yamls2go.go | 2 +- pkg/metrics/service-monitor.go | 3 +-- pkg/names/names.go | 4 +--- 15 files changed, 21 insertions(+), 30 deletions(-) diff --git a/controllers/metrics/metrics.go b/controllers/metrics/metrics.go index 95b2331b5..1994ccdb1 100644 --- a/controllers/metrics/metrics.go +++ b/controllers/metrics/metrics.go @@ -21,7 +21,6 @@ package metrics import ( "context" "errors" - "fmt" "github.com/go-logr/logr" "github.com/submariner-io/submariner-operator/controllers/apply" @@ -94,7 +93,7 @@ func newMetricsService(name, namespace, appKey, appName string, port int32) *cor ObjectMeta: metav1.ObjectMeta{ Labels: labels, Namespace: namespace, - Name: fmt.Sprintf("%s-metrics", name), + Name: name + "-metrics", }, Spec: corev1.ServiceSpec{ Ports: servicePorts, diff --git a/controllers/submariner/gateway_resources.go b/controllers/submariner/gateway_resources.go index 0e6859ee2..388500382 100644 --- a/controllers/submariner/gateway_resources.go +++ b/controllers/submariner/gateway_resources.go @@ -20,7 +20,6 @@ package submariner import ( "context" - "fmt" "strconv" "github.com/go-logr/logr" @@ -123,7 +122,7 @@ func newGatewayPodTemplate(cr *v1alpha1.Submariner, name string, podSelectorLabe // We've got a PSK secret, mount it where the gateway expects it volumeMounts = append(volumeMounts, corev1.VolumeMount{ Name: "psksecret", - MountPath: fmt.Sprintf("/var/run/secrets/submariner.io/%s", cr.Spec.CeIPSecPSKSecret), + MountPath: "/var/run/secrets/submariner.io/" + cr.Spec.CeIPSecPSKSecret, ReadOnly: true, }) diff --git a/controllers/submariner/loadbalancer_resources.go b/controllers/submariner/loadbalancer_resources.go index c29ca97ef..3aacfd107 100644 --- a/controllers/submariner/loadbalancer_resources.go +++ b/controllers/submariner/loadbalancer_resources.go @@ -20,7 +20,7 @@ package submariner import ( "context" - "fmt" + "strconv" "github.com/go-logr/logr" configv1 "github.com/openshift/api/config/v1" @@ -61,7 +61,7 @@ func (r *Reconciler) reconcileLoadBalancer( // For IBM cloud also needs to annotate the allocated health check node port if platformTypeOCP == string(configv1.IBMCloudPlatformType) { - healthPortStr := fmt.Sprintf("%d", svc.Spec.HealthCheckNodePort) + healthPortStr := strconv.Itoa(int(svc.Spec.HealthCheckNodePort)) svc.ObjectMeta.Annotations = map[string]string{ "service.kubernetes.io/ibm-load-balancer-cloud-provider-vpc-health-check-port": healthPortStr, } diff --git a/controllers/submariner/metrics_proxy_resources.go b/controllers/submariner/metrics_proxy_resources.go index a6213564b..fc9266633 100644 --- a/controllers/submariner/metrics_proxy_resources.go +++ b/controllers/submariner/metrics_proxy_resources.go @@ -20,7 +20,7 @@ package submariner import ( "context" - "fmt" + "strconv" "github.com/go-logr/logr" "github.com/submariner-io/admiral/pkg/names" @@ -63,7 +63,8 @@ func newMetricsProxyDaemonSet(cr *v1alpha1.Submariner) *appsv1.DaemonSet { }, Spec: corev1.PodSpec{ Containers: []corev1.Container{ - *metricProxyContainer(cr, "gateway-metrics-proxy", fmt.Sprint(gatewayMetricsServicePort), gatewayMetricsServerPort), + *metricProxyContainer(cr, "gateway-metrics-proxy", strconv.Itoa(gatewayMetricsServicePort), + gatewayMetricsServerPort), }, NodeSelector: map[string]string{"submariner.io/gateway": "true"}, // The MetricsProxy Pod must be able to run on any flagged node, regardless of existing taints @@ -75,7 +76,7 @@ func newMetricsProxyDaemonSet(cr *v1alpha1.Submariner) *appsv1.DaemonSet { if cr.Spec.GlobalCIDR != "" { daemonSet.Spec.Template.Spec.Containers = append(daemonSet.Spec.Template.Spec.Containers, - *metricProxyContainer(cr, "globalnet-metrics-proxy", fmt.Sprint(globalnetMetricsServicePort), globalnetMetricsServerPort)) + *metricProxyContainer(cr, "globalnet-metrics-proxy", strconv.Itoa(globalnetMetricsServicePort), globalnetMetricsServerPort)) } return daemonSet diff --git a/controllers/submariner/submariner_controller_test.go b/controllers/submariner/submariner_controller_test.go index ccb45f641..61a27a548 100644 --- a/controllers/submariner/submariner_controller_test.go +++ b/controllers/submariner/submariner_controller_test.go @@ -20,7 +20,6 @@ package submariner_test import ( "context" - "fmt" "os" "reflect" "time" @@ -287,7 +286,7 @@ func testReconciliation() { saName := opnames.ForClusterSA(t.submariner.Spec.ClusterID) brokerSecret = &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("%s-token", saName), + Name: saName + "-token", Namespace: t.submariner.Spec.BrokerK8sRemoteNamespace, Annotations: map[string]string{ corev1.ServiceAccountNameKey: saName, diff --git a/controllers/submariner/submariner_networkdiscovery.go b/controllers/submariner/submariner_networkdiscovery.go index 5c804ba53..76884790f 100644 --- a/controllers/submariner/submariner_networkdiscovery.go +++ b/controllers/submariner/submariner_networkdiscovery.go @@ -20,7 +20,6 @@ package submariner import ( "context" - "fmt" "github.com/go-logr/logr" "github.com/pkg/errors" @@ -90,7 +89,7 @@ func getCIDR(log logr.Logger, cidrType, currentCIDR string, detectedCIDRs []stri if detected != "" && detected != currentCIDR { log.Error( - fmt.Errorf("there is a mismatch between the detected and configured CIDRs"), + errors.New("there is a mismatch between the detected and configured CIDRs"), "The configured CIDR will take precedence", "type", cidrType, "configured", currentCIDR, "detected", detected) } @@ -102,7 +101,7 @@ func getFirstCIDR(detectedCIDRs []string) string { CIDRlen := len(detectedCIDRs) if CIDRlen > 1 { - log.Error(fmt.Errorf("detected > 1 CIDRs"), + log.Error(errors.New("detected > 1 CIDRs"), "we currently support only one", "detectedCIDRs", detectedCIDRs) } diff --git a/main.go b/main.go index 84706fc71..27fd7e4a7 100644 --- a/main.go +++ b/main.go @@ -70,7 +70,7 @@ var ( ) func printVersion() { - log.Info(fmt.Sprintf("Go Version: %s", runtime.Version())) + log.Info("Go Version: " + runtime.Version()) log.Info(fmt.Sprintf("Go OS/Arch: %s/%s", runtime.GOOS, runtime.GOARCH)) } diff --git a/pkg/discovery/clustersetip/clustersetip.go b/pkg/discovery/clustersetip/clustersetip.go index 99b2c14dc..8940eb7dc 100644 --- a/pkg/discovery/clustersetip/clustersetip.go +++ b/pkg/discovery/clustersetip/clustersetip.go @@ -21,7 +21,6 @@ package clustersetip import ( "context" "encoding/json" - "fmt" "github.com/pkg/errors" "github.com/submariner-io/admiral/pkg/reporter" @@ -122,7 +121,7 @@ func assignClustersetIPs(clustersetIPInfo *Info, netconfig Config, status report return "", status.Error(err, "unable to allocate clustersetip CIDR") } - status.Success(fmt.Sprintf("Allocated clustersetip CIDR %s", clustersetIPCIDR)) + status.Success("Allocated clustersetip CIDR " + clustersetIPCIDR) } } else { // ClustersetIP enabled, clustersetIPCIDR specified by user diff --git a/pkg/discovery/clustersetip/config_map.go b/pkg/discovery/clustersetip/config_map.go index 5cecdfe4b..ce5e57509 100644 --- a/pkg/discovery/clustersetip/config_map.go +++ b/pkg/discovery/clustersetip/config_map.go @@ -21,7 +21,6 @@ package clustersetip import ( "context" "encoding/json" - "fmt" "strconv" "github.com/pkg/errors" @@ -70,7 +69,7 @@ func NewClustersetIPConfigMap(clustersetIPEnabled bool, defaultClusteretIPCidrRa data := map[string]string{ clustersetIPEnabledKey: strconv.FormatBool(clustersetIPEnabled), clustersetIPCidrRange: string(cidrRange), - clustersetIPClusterSize: fmt.Sprint(defaultClustersetIPClusterSize), + clustersetIPClusterSize: strconv.FormatUint(uint64(defaultClustersetIPClusterSize), 10), } cm := &corev1.ConfigMap{ diff --git a/pkg/discovery/globalnet/config_map.go b/pkg/discovery/globalnet/config_map.go index 29ef719e2..a3b150428 100644 --- a/pkg/discovery/globalnet/config_map.go +++ b/pkg/discovery/globalnet/config_map.go @@ -21,7 +21,7 @@ package globalnet import ( "context" "encoding/json" - "fmt" + "strconv" "github.com/pkg/errors" "github.com/submariner-io/submariner-operator/pkg/cidr" @@ -74,7 +74,7 @@ func NewGlobalnetConfigMap(globalnetEnabled bool, defaultGlobalCidrRange string, data = map[string]string{ globalnetEnabledKey: "true", globalnetCidrRange: string(cidrRange), - globalnetClusterSize: fmt.Sprint(defaultGlobalClusterSize), + globalnetClusterSize: strconv.FormatUint(uint64(defaultGlobalClusterSize), 10), } } else { data = map[string]string{ diff --git a/pkg/discovery/globalnet/globalnet.go b/pkg/discovery/globalnet/globalnet.go index ccc15df03..5f337d223 100644 --- a/pkg/discovery/globalnet/globalnet.go +++ b/pkg/discovery/globalnet/globalnet.go @@ -21,7 +21,6 @@ package globalnet import ( "context" "encoding/json" - "fmt" "github.com/pkg/errors" "github.com/submariner-io/admiral/pkg/reporter" @@ -152,7 +151,7 @@ func AssignGlobalnetIPs(globalnetInfo *Info, netconfig Config, status reporter.I return "", status.Error(err, "unable to allocate global CIDR") } - status.Success(fmt.Sprintf("Allocated global CIDR %s", globalnetCIDR)) + status.Success("Allocated global CIDR " + globalnetCIDR) } } else { // Globalnet enabled, globalnetCIDR specified by user diff --git a/pkg/discovery/network/generic.go b/pkg/discovery/network/generic.go index 6e669f34f..3575c5fc2 100644 --- a/pkg/discovery/network/generic.go +++ b/pkg/discovery/network/generic.go @@ -126,7 +126,7 @@ func findClusterIPRangeFromServiceCreation(ctx context.Context, client controlle // creating invalid service didn't fail as expected if err == nil { - return "", fmt.Errorf("could not determine the service IP range via service creation - " + + return "", errors.New("could not determine the service IP range via service creation - " + "expected a specific error but none was returned") } diff --git a/pkg/embeddedyamls/generators/yamls2go.go b/pkg/embeddedyamls/generators/yamls2go.go index da30d0e22..e5403d676 100644 --- a/pkg/embeddedyamls/generators/yamls2go.go +++ b/pkg/embeddedyamls/generators/yamls2go.go @@ -169,7 +169,7 @@ const ( for _, index := range reDoc.FindAllIndex(contents, 2) { if index[0] > 0 { // Document starting inside the contents - panic(fmt.Sprintf("%s contains more than one document, use one file per document", f)) + panic(f + " contains more than one document, use one file per document") } } diff --git a/pkg/metrics/service-monitor.go b/pkg/metrics/service-monitor.go index dc941a935..4a6f497de 100644 --- a/pkg/metrics/service-monitor.go +++ b/pkg/metrics/service-monitor.go @@ -19,7 +19,6 @@ package metrics import ( "context" - "fmt" "github.com/pkg/errors" monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" @@ -32,7 +31,7 @@ import ( "k8s.io/utils/ptr" ) -var ErrServiceMonitorNotPresent = fmt.Errorf("no ServiceMonitor registered with the API") +var ErrServiceMonitorNotPresent = errors.New("no ServiceMonitor registered with the API") const openshiftMonitoringNS = "openshift-monitoring" diff --git a/pkg/names/names.go b/pkg/names/names.go index 4c1be1e36..25c2f3f2c 100644 --- a/pkg/names/names.go +++ b/pkg/names/names.go @@ -18,8 +18,6 @@ limitations under the License. package names -import "fmt" - /* CR names and other constants. */ const ( ServiceDiscoveryCrName = "service-discovery" @@ -45,5 +43,5 @@ func AppendUninstall(name string) string { } func ForClusterSA(clusterID string) string { - return fmt.Sprintf("cluster-%s", clusterID) + return "cluster-" + clusterID } From 45c33c3c994b235c52c92591e40f43dbc4e2d83e Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Wed, 27 Nov 2024 13:16:27 -0500 Subject: [PATCH 5/5] Address maintidx linter violations "Function name: testReconciliation, Cyclomatic Complexity: 2, Halstead Volume: 13318.79, Maintainability Index: 14" Refactored testReconciliation into several functions to alleviate the complexity. Signed-off-by: Tom Pantelis --- .../submariner/submariner_controller_test.go | 228 ++++++++++-------- 1 file changed, 125 insertions(+), 103 deletions(-) diff --git a/controllers/submariner/submariner_controller_test.go b/controllers/submariner/submariner_controller_test.go index 61a27a548..c2cc65c93 100644 --- a/controllers/submariner/submariner_controller_test.go +++ b/controllers/submariner/submariner_controller_test.go @@ -54,7 +54,13 @@ const ( ) var _ = Describe("Submariner controller tests", func() { - Context("Reconciliation", testReconciliation) + Context("Reconciliation", func() { + testSubmarinerResourceReconciliation() + testDaemonSetReconciliation() + testServiceDiscoveryReconciliation() + testLoadBalancerReconciliation() + testBrokerSecretReconciliation() + }) When("the Submariner resource is being deleted", testDeletion) }) @@ -65,7 +71,7 @@ const ( testConfiguredClusterCIDR = "192.168.67.0/24" ) -func testReconciliation() { +func testSubmarinerResourceReconciliation() { t := newTestDriver() It("should add a finalizer to the Submariner resource", func(ctx SpecContext) { @@ -120,6 +126,49 @@ func testReconciliation() { }) }) + When("the Submariner resource doesn't exist", func() { + BeforeEach(func() { + t.InitScopedClientObjs = nil + }) + + It("should return success without creating any resources", func(ctx SpecContext) { + t.AssertReconcileSuccess(ctx) + t.AssertNoDaemonSet(ctx, names.GatewayComponent) + t.AssertNoDaemonSet(ctx, names.RouteAgentComponent) + }) + }) + + When("the Submariner resource is missing values for certain fields", func() { + BeforeEach(func() { + t.submariner.Spec.Repository = "" + t.submariner.Spec.Version = "" + }) + + It("should update the resource with defaults", func(ctx SpecContext) { + t.AssertReconcileSuccess(ctx) + + updated := t.getSubmariner(ctx) + Expect(updated.Spec.Repository).To(Equal(v1alpha1.DefaultRepo)) + Expect(updated.Spec.Version).To(Equal(v1alpha1.DefaultSubmarinerVersion)) + }) + }) + + When("Submariner resource retrieval fails", func() { + BeforeEach(func() { + t.ScopedClient = fake.NewReactingClient(t.NewScopedClient()).AddReactor(fake.Get, &v1alpha1.Submariner{}, + fake.FailingReaction(nil)) + }) + + It("should return an error", func(ctx SpecContext) { + _, err := t.DoReconcile(ctx) + Expect(err).To(HaveOccurred()) + }) + }) +} + +func testDaemonSetReconciliation() { + t := newTestDriver() + When("the submariner gateway DaemonSet doesn't exist", func() { It("should create it", func(ctx SpecContext) { t.AssertReconcileSuccess(ctx) @@ -205,6 +254,72 @@ func testReconciliation() { }) }) + When("DaemonSet creation fails", func() { + BeforeEach(func() { + t.ScopedClient = fake.NewReactingClient(t.NewScopedClient()).AddReactor(fake.Create, &appsv1.DaemonSet{}, + fake.FailingReaction(nil)) + }) + + It("should return an error", func(ctx SpecContext) { + _, err := t.DoReconcile(ctx) + Expect(err).To(HaveOccurred()) + }) + }) + + When("DaemonSet retrieval fails", func() { + BeforeEach(func() { + t.ScopedClient = fake.NewReactingClient(t.NewScopedClient()).AddReactor(fake.Get, &appsv1.DaemonSet{}, + fake.FailingReaction(nil)) + }) + + It("should return an error", func(ctx SpecContext) { + _, err := t.DoReconcile(ctx) + Expect(err).To(HaveOccurred()) + }) + }) + + When("proxy environment variables are set", func() { + var httpProxy, httpsProxy, noProxy string + var httpProxySet, httpsProxySet, noProxySet bool + const testHTTPSProxy = "https://proxy.example.com" + const testHTTPProxy = "http://proxy.example.com" + const testNoProxy = "127.0.0.1" + + BeforeEach(func() { + // We know we only write the all-caps versions + httpProxy, httpProxySet = os.LookupEnv("HTTP_PROXY") + httpsProxy, httpsProxySet = os.LookupEnv("HTTPS_PROXY") + noProxy, noProxySet = os.LookupEnv("NO_PROXY") + os.Setenv("HTTPS_PROXY", testHTTPSProxy) + os.Setenv("HTTP_PROXY", testHTTPProxy) + os.Setenv("NO_PROXY", testNoProxy) + }) + + AfterEach(func() { + restoreOrUnsetEnv("HTTPS_PROXY", httpsProxySet, httpsProxy) + restoreOrUnsetEnv("HTTP_PROXY", httpProxySet, httpProxy) + restoreOrUnsetEnv("NO_PROXY", noProxySet, noProxy) + }) + + It("should populate them in generated container specs", func(ctx SpecContext) { + t.AssertReconcileSuccess(ctx) + + for _, component := range []string{ + names.GatewayComponent, names.GlobalnetComponent, names.MetricsProxyComponent, names.RouteAgentComponent, + } { + daemonSet := t.AssertDaemonSet(ctx, component) + envMap := test.EnvMapFrom(daemonSet) + Expect(envMap).To(HaveKeyWithValue("HTTPS_PROXY", testHTTPSProxy)) + Expect(envMap).To(HaveKeyWithValue("HTTP_PROXY", testHTTPProxy)) + Expect(envMap).To(HaveKeyWithValue("NO_PROXY", testNoProxy)) + } + }) + }) +} + +func testServiceDiscoveryReconciliation() { + t := newTestDriver() + When("ServiceDiscovery is enabled", func() { BeforeEach(func() { t.submariner.Spec.ServiceDiscoveryEnabled = true @@ -230,6 +345,10 @@ func testReconciliation() { Expect(serviceDiscovery.Spec.GlobalnetEnabled).To(BeTrue()) }) }) +} + +func testLoadBalancerReconciliation() { + t := newTestDriver() When("load balancer is enabled", func() { BeforeEach(func() { @@ -268,6 +387,10 @@ func testReconciliation() { }) }) }) +} + +func testBrokerSecretReconciliation() { + t := newTestDriver() When("the broker secret is created/updated", func() { var brokerSecret *corev1.Secret @@ -347,107 +470,6 @@ func testReconciliation() { }) }) }) - - When("the Submariner resource doesn't exist", func() { - BeforeEach(func() { - t.InitScopedClientObjs = nil - }) - - It("should return success without creating any resources", func(ctx SpecContext) { - t.AssertReconcileSuccess(ctx) - t.AssertNoDaemonSet(ctx, names.GatewayComponent) - t.AssertNoDaemonSet(ctx, names.RouteAgentComponent) - }) - }) - - When("the Submariner resource is missing values for certain fields", func() { - BeforeEach(func() { - t.submariner.Spec.Repository = "" - t.submariner.Spec.Version = "" - }) - - It("should update the resource with defaults", func(ctx SpecContext) { - t.AssertReconcileSuccess(ctx) - - updated := t.getSubmariner(ctx) - Expect(updated.Spec.Repository).To(Equal(v1alpha1.DefaultRepo)) - Expect(updated.Spec.Version).To(Equal(v1alpha1.DefaultSubmarinerVersion)) - }) - }) - - When("DaemonSet creation fails", func() { - BeforeEach(func() { - t.ScopedClient = fake.NewReactingClient(t.NewScopedClient()).AddReactor(fake.Create, &appsv1.DaemonSet{}, - fake.FailingReaction(nil)) - }) - - It("should return an error", func(ctx SpecContext) { - _, err := t.DoReconcile(ctx) - Expect(err).To(HaveOccurred()) - }) - }) - - When("DaemonSet retrieval fails", func() { - BeforeEach(func() { - t.ScopedClient = fake.NewReactingClient(t.NewScopedClient()).AddReactor(fake.Get, &appsv1.DaemonSet{}, - fake.FailingReaction(nil)) - }) - - It("should return an error", func(ctx SpecContext) { - _, err := t.DoReconcile(ctx) - Expect(err).To(HaveOccurred()) - }) - }) - - When("Submariner resource retrieval fails", func() { - BeforeEach(func() { - t.ScopedClient = fake.NewReactingClient(t.NewScopedClient()).AddReactor(fake.Get, &v1alpha1.Submariner{}, - fake.FailingReaction(nil)) - }) - - It("should return an error", func(ctx SpecContext) { - _, err := t.DoReconcile(ctx) - Expect(err).To(HaveOccurred()) - }) - }) - - When("proxy environment variables are set", func() { - var httpProxy, httpsProxy, noProxy string - var httpProxySet, httpsProxySet, noProxySet bool - const testHTTPSProxy = "https://proxy.example.com" - const testHTTPProxy = "http://proxy.example.com" - const testNoProxy = "127.0.0.1" - - BeforeEach(func() { - // We know we only write the all-caps versions - httpProxy, httpProxySet = os.LookupEnv("HTTP_PROXY") - httpsProxy, httpsProxySet = os.LookupEnv("HTTPS_PROXY") - noProxy, noProxySet = os.LookupEnv("NO_PROXY") - os.Setenv("HTTPS_PROXY", testHTTPSProxy) - os.Setenv("HTTP_PROXY", testHTTPProxy) - os.Setenv("NO_PROXY", testNoProxy) - }) - - AfterEach(func() { - restoreOrUnsetEnv("HTTPS_PROXY", httpsProxySet, httpsProxy) - restoreOrUnsetEnv("HTTP_PROXY", httpProxySet, httpProxy) - restoreOrUnsetEnv("NO_PROXY", noProxySet, noProxy) - }) - - It("should populate them in generated container specs", func(ctx SpecContext) { - t.AssertReconcileSuccess(ctx) - - for _, component := range []string{ - names.GatewayComponent, names.GlobalnetComponent, names.MetricsProxyComponent, names.RouteAgentComponent, - } { - daemonSet := t.AssertDaemonSet(ctx, component) - envMap := test.EnvMapFrom(daemonSet) - Expect(envMap).To(HaveKeyWithValue("HTTPS_PROXY", testHTTPSProxy)) - Expect(envMap).To(HaveKeyWithValue("HTTP_PROXY", testHTTPProxy)) - Expect(envMap).To(HaveKeyWithValue("NO_PROXY", testNoProxy)) - } - }) - }) } func restoreOrUnsetEnv(envVar string, wasSet bool, value string) {