From 6a0b71c9675786494c067507e6ab74a1adbef574 Mon Sep 17 00:00:00 2001 From: Ricardo Maraschini Date: Thu, 10 Feb 2022 11:35:54 +0100 Subject: [PATCH] env: turning 'base' a component (PROJQUAY-3055) To avoid having an Override reference directly into QuayRegitry's Spec we need to turn "base" into a component. This PR also adds e2e tests for the env var override feature. --- apis/quay/v1/quayregistry_types.go | 103 ++++++++++++------ apis/quay/v1/quayregistry_types_test.go | 16 ++- apis/quay/v1/zz_generated.deepcopy.go | 5 - bundle/manifests/quayregistries.crd.yaml | 89 --------------- .../manifests/quayregistries.crd.yaml | 89 --------------- .../bases/quay.redhat.com_quayregistries.yaml | 89 --------------- controllers/quay/quayregistry_controller.go | 11 +- .../quay/quayregistry_controller_test.go | 9 +- e2e/base_component/00-assert.yaml | 26 +++++ .../00-create-quay-registry.yaml | 26 +++++ e2e/environment_vars/00-assert.yaml | 86 +++++++++++++++ .../00-create-quay-registry.yaml | 36 ++++++ e2e/min_deployment/00-assert.yaml | 2 + pkg/kustomize/kustomize.go | 2 +- pkg/kustomize/secrets.go | 15 +++ pkg/middleware/middleware.go | 7 -- 16 files changed, 289 insertions(+), 322 deletions(-) create mode 100644 e2e/base_component/00-assert.yaml create mode 100644 e2e/base_component/00-create-quay-registry.yaml create mode 100644 e2e/environment_vars/00-assert.yaml create mode 100644 e2e/environment_vars/00-create-quay-registry.yaml diff --git a/apis/quay/v1/quayregistry_types.go b/apis/quay/v1/quayregistry_types.go index f0729519d..a2cd4785c 100644 --- a/apis/quay/v1/quayregistry_types.go +++ b/apis/quay/v1/quayregistry_types.go @@ -17,7 +17,6 @@ limitations under the License. package v1 import ( - "errors" "fmt" "os" "strings" @@ -52,6 +51,7 @@ const ( ) var allComponents = []ComponentKind{ + ComponentBase, ComponentPostgres, ComponentClair, ComponentRedis, @@ -78,6 +78,7 @@ var supportsVolumeOverride = []ComponentKind{ } var supportsEnvOverride = []ComponentKind{ + ComponentBase, ComponentClair, ComponentMirror, ComponentPostgres, @@ -97,8 +98,6 @@ type QuayRegistrySpec struct { ConfigBundleSecret string `json:"configBundleSecret,omitempty"` // Components declare how the Operator should handle backing Quay services. Components []Component `json:"components,omitempty"` - // Overrides holds overrides for the Base component (quay-app). - Overrides *Override `json:"overrides,omitempty"` } // Component describes how the Operator should handle a backing Quay service. @@ -337,58 +336,94 @@ func RequiredComponent(component ComponentKind) bool { } // EnsureDefaultComponents adds any `Components` which are missing from `Spec.Components`. -// Returns an error if a component was declared as managed but is not supported in the current k8s cluster. -func EnsureDefaultComponents(ctx *quaycontext.QuayRegistryContext, quay *QuayRegistry) (*QuayRegistry, error) { - updatedQuay := quay.DeepCopy() - if updatedQuay.Spec.Components == nil { - updatedQuay.Spec.Components = []Component{} +// Returns an error if a component was declared as managed but is not supported in the current +// k8s cluster. +func EnsureDefaultComponents(ctx *quaycontext.QuayRegistryContext, quay *QuayRegistry) error { + if quay.Spec.Components == nil { + quay.Spec.Components = []Component{} } - type componentCheck struct { + type check struct { check func() bool msg string } - componentChecks := map[ComponentKind]componentCheck{ - ComponentRoute: {func() bool { return ctx.SupportsRoutes }, "cannot use `route` component when `Route` API not available"}, - ComponentTLS: {func() bool { return ctx.SupportsRoutes }, "cannot use `tls` component when `Route` API not available"}, - ComponentObjectStorage: {func() bool { return ctx.SupportsObjectStorage }, "cannot use `ObjectStorage` component when `ObjectStorage` API not available"}, - ComponentMonitoring: {func() bool { return ctx.SupportsMonitoring }, "cannot use `monitoring` component when `Prometheus` API not available"}, + checks := map[ComponentKind]check{ + ComponentRoute: { + check: func() bool { return ctx.SupportsRoutes }, + msg: "Route API not available", + }, + ComponentTLS: { + check: func() bool { return ctx.SupportsRoutes }, + msg: "Route API not available", + }, + ComponentObjectStorage: { + check: func() bool { return ctx.SupportsObjectStorage }, + msg: "ObjectStorage API not available", + }, + ComponentMonitoring: { + check: func() bool { return ctx.SupportsMonitoring }, + msg: "Prometheus API not available", + }, } - componentManaged := map[ComponentKind]componentCheck{ + componentManaged := map[ComponentKind]check{ ComponentTLS: { check: func() bool { return ctx.TLSCert == nil && ctx.TLSKey == nil }, }, } - for _, component := range allComponents { - componentCheck, checkExists := componentChecks[component] - if (checkExists && !componentCheck.check()) && ComponentIsManaged(quay.Spec.Components, component) { - return quay, errors.New(componentCheck.msg) + for _, cmp := range allComponents { + ccheck, checkexists := checks[cmp] + if checkexists { + // if there is a check registered for the component we run it, if the + // check fails and the component is managed then we have a problem with + // the current components configuration. returns the check error. + if !ccheck.check() && ComponentIsManaged(quay.Spec.Components, cmp) { + return fmt.Errorf( + "Error validatingcomponent %s: %s", cmp, ccheck.msg, + ) + } } - found := false - for _, declaredComponent := range quay.Spec.Components { - if component == declaredComponent.Kind { - found = true - break + // if the component has already been declared in the QuayRegistry object we can + // just return as there is nothing we need to do. + var found bool + for i, declaredComponent := range quay.Spec.Components { + if cmp != declaredComponent.Kind { + continue + } + + // we disregard whatever the user has defined for base component, this + // is a component that can't be unmanaged so if user sets it to unmanaged + // we are going to roll it back to managed. + if declaredComponent.Kind == ComponentBase { + quay.Spec.Components[i].Managed = true } + + found = true + break + } + if found { + continue } - managed := !checkExists || componentCheck.check() - if _, ok := componentManaged[component]; ok { - managed = managed && componentManaged[component].check() + // the component management status is set to true if the check for the component + // has passed. + managed := !checkexists || ccheck.check() + if _, ok := componentManaged[cmp]; ok { + managed = managed && componentManaged[cmp].check() } - if !found { - updatedQuay.Spec.Components = append(updatedQuay.Spec.Components, Component{ - Kind: component, + quay.Spec.Components = append( + quay.Spec.Components, + Component{ + Kind: cmp, Managed: managed, - }) - } + }, + ) } - return updatedQuay, nil + return nil } // ValidateOverrides validates that the overrides set for each component are valid. @@ -580,6 +615,8 @@ func FieldGroupNameFor(cmp ComponentKind) (string, error) { return "", nil case ComponentTLS: return "", nil + case ComponentBase: + return "", nil default: return "", fmt.Errorf("unknown component: %q", cmp) } diff --git a/apis/quay/v1/quayregistry_types_test.go b/apis/quay/v1/quayregistry_types_test.go index f3a321a44..244ee55e0 100644 --- a/apis/quay/v1/quayregistry_types_test.go +++ b/apis/quay/v1/quayregistry_types_test.go @@ -23,6 +23,7 @@ var ensureDefaultComponentsTests = []struct { QuayRegistry{ Spec: QuayRegistrySpec{ Components: []Component{ + {Kind: "base", Managed: true}, {Kind: "postgres", Managed: true}, {Kind: "redis", Managed: true}, {Kind: "clair", Managed: true}, @@ -42,6 +43,7 @@ var ensureDefaultComponentsTests = []struct { SupportsRoutes: true, }, []Component{ + {Kind: "base", Managed: true}, {Kind: "postgres", Managed: true}, {Kind: "redis", Managed: true}, {Kind: "clair", Managed: true}, @@ -60,6 +62,7 @@ var ensureDefaultComponentsTests = []struct { QuayRegistry{ Spec: QuayRegistrySpec{ Components: []Component{ + {Kind: "base", Managed: true}, {Kind: "postgres", Managed: true}, {Kind: "redis", Managed: true}, {Kind: "clair", Managed: true}, @@ -74,6 +77,7 @@ var ensureDefaultComponentsTests = []struct { }, quaycontext.QuayRegistryContext{}, []Component{ + {Kind: "base", Managed: true}, {Kind: "postgres", Managed: true}, {Kind: "redis", Managed: true}, {Kind: "clair", Managed: true}, @@ -91,6 +95,7 @@ var ensureDefaultComponentsTests = []struct { QuayRegistry{ Spec: QuayRegistrySpec{ Components: []Component{ + {Kind: "base", Managed: true}, {Kind: "postgres", Managed: true}, {Kind: "redis", Managed: true}, {Kind: "clair", Managed: true}, @@ -110,6 +115,7 @@ var ensureDefaultComponentsTests = []struct { SupportsMonitoring: true, }, []Component{ + {Kind: "base", Managed: true}, {Kind: "postgres", Managed: true}, {Kind: "redis", Managed: true}, {Kind: "clair", Managed: true}, @@ -128,6 +134,7 @@ var ensureDefaultComponentsTests = []struct { QuayRegistry{ Spec: QuayRegistrySpec{ Components: []Component{ + {Kind: "base", Managed: true}, {Kind: "postgres", Managed: true}, {Kind: "redis", Managed: true}, {Kind: "clair", Managed: true}, @@ -147,6 +154,7 @@ var ensureDefaultComponentsTests = []struct { SupportsMonitoring: true, }, []Component{ + {Kind: "base", Managed: true}, {Kind: "postgres", Managed: true}, {Kind: "redis", Managed: true}, {Kind: "clair", Managed: true}, @@ -167,6 +175,7 @@ var ensureDefaultComponentsTests = []struct { }, quaycontext.QuayRegistryContext{}, []Component{ + {Kind: "base", Managed: true}, {Kind: "postgres", Managed: true}, {Kind: "redis", Managed: true}, {Kind: "clair", Managed: true}, @@ -191,6 +200,7 @@ var ensureDefaultComponentsTests = []struct { SupportsMonitoring: true, }, []Component{ + {Kind: "base", Managed: true}, {Kind: "postgres", Managed: true}, {Kind: "redis", Managed: true}, {Kind: "clair", Managed: true}, @@ -217,6 +227,7 @@ var ensureDefaultComponentsTests = []struct { }, quaycontext.QuayRegistryContext{}, []Component{ + {Kind: "base", Managed: true}, {Kind: "postgres", Managed: false}, {Kind: "redis", Managed: true}, {Kind: "clair", Managed: true}, @@ -247,6 +258,7 @@ var ensureDefaultComponentsTests = []struct { ClusterHostname: "apps.example.com", }, []Component{ + {Kind: "base", Managed: true}, {Kind: "postgres", Managed: false}, {Kind: "redis", Managed: true}, {Kind: "clair", Managed: true}, @@ -279,6 +291,7 @@ var ensureDefaultComponentsTests = []struct { ClusterHostname: "apps.example.com", }, []Component{ + {Kind: "base", Managed: true}, {Kind: "postgres", Managed: false}, {Kind: "redis", Managed: true}, {Kind: "clair", Managed: true}, @@ -298,7 +311,8 @@ func TestEnsureDefaultComponents(t *testing.T) { assert := assert.New(t) for _, test := range ensureDefaultComponentsTests { - updatedQuay, err := EnsureDefaultComponents(&test.ctx, &test.quay) + updatedQuay := &test.quay + err := EnsureDefaultComponents(&test.ctx, &test.quay) if test.expectedErr != nil { assert.NotNil(err, test.name) diff --git a/apis/quay/v1/zz_generated.deepcopy.go b/apis/quay/v1/zz_generated.deepcopy.go index 4f840e2e8..9936206d3 100644 --- a/apis/quay/v1/zz_generated.deepcopy.go +++ b/apis/quay/v1/zz_generated.deepcopy.go @@ -159,11 +159,6 @@ func (in *QuayRegistrySpec) DeepCopyInto(out *QuayRegistrySpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.Overrides != nil { - in, out := &in.Overrides, &out.Overrides - *out = new(Override) - (*in).DeepCopyInto(*out) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new QuayRegistrySpec. diff --git a/bundle/manifests/quayregistries.crd.yaml b/bundle/manifests/quayregistries.crd.yaml index e2553ebcb..d26d00a7f 100644 --- a/bundle/manifests/quayregistries.crd.yaml +++ b/bundle/manifests/quayregistries.crd.yaml @@ -140,95 +140,6 @@ spec: configBundleSecret: description: ConfigBundleSecret is the name of the Kubernetes `Secret` in the same namespace which contains the base Quay config and extra certs. type: string - overrides: - description: Overrides holds overrides for the Base component (quay-app). - properties: - env: - items: - description: EnvVar represents an environment variable present in a Container. - properties: - name: - description: Name of the environment variable. Must be a C_IDENTIFIER. - type: string - value: - description: 'Variable references $(VAR_NAME) are expanded using the previously defined environment variables in the container and any service environment variables. If a variable cannot be resolved, the reference in the input string will be unchanged. Double $$ are reduced to a single $, which allows for escaping the $(VAR_NAME) syntax: i.e. "$$(VAR_NAME)" will produce the string literal "$(VAR_NAME)". Escaped references will never be expanded, regardless of whether the variable exists or not. Defaults to "".' - type: string - valueFrom: - description: Source for the environment variable's value. Cannot be used if value is not empty. - properties: - configMapKeyRef: - description: Selects a key of a ConfigMap. - properties: - key: - description: The key to select. - type: string - name: - description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?' - type: string - optional: - description: Specify whether the ConfigMap or its key must be defined - type: boolean - required: - - key - type: object - fieldRef: - description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, `metadata.annotations['''']`, spec.nodeName, spec.serviceAccountName, status.hostIP, status.podIP, status.podIPs.' - properties: - apiVersion: - description: Version of the schema the FieldPath is written in terms of, defaults to "v1". - type: string - fieldPath: - description: Path of the field to select in the specified API version. - type: string - required: - - fieldPath - type: object - resourceFieldRef: - description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, limits.ephemeral-storage, requests.cpu, requests.memory and requests.ephemeral-storage) are currently supported.' - properties: - containerName: - description: 'Container name: required for volumes, optional for env vars' - type: string - divisor: - anyOf: - - type: integer - - type: string - description: Specifies the output format of the exposed resources, defaults to "1" - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - resource: - description: 'Required: resource to select' - type: string - required: - - resource - type: object - secretKeyRef: - description: Selects a key of a secret in the pod's namespace - properties: - key: - description: The key of the secret to select from. Must be a valid secret key. - type: string - name: - description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?' - type: string - optional: - description: Specify whether the Secret or its key must be defined - type: boolean - required: - - key - type: object - type: object - required: - - name - type: object - type: array - volumeSize: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - type: object type: object status: description: QuayRegistryStatus defines the observed state of QuayRegistry. diff --git a/bundle/upstream/manifests/quayregistries.crd.yaml b/bundle/upstream/manifests/quayregistries.crd.yaml index e2553ebcb..d26d00a7f 100644 --- a/bundle/upstream/manifests/quayregistries.crd.yaml +++ b/bundle/upstream/manifests/quayregistries.crd.yaml @@ -140,95 +140,6 @@ spec: configBundleSecret: description: ConfigBundleSecret is the name of the Kubernetes `Secret` in the same namespace which contains the base Quay config and extra certs. type: string - overrides: - description: Overrides holds overrides for the Base component (quay-app). - properties: - env: - items: - description: EnvVar represents an environment variable present in a Container. - properties: - name: - description: Name of the environment variable. Must be a C_IDENTIFIER. - type: string - value: - description: 'Variable references $(VAR_NAME) are expanded using the previously defined environment variables in the container and any service environment variables. If a variable cannot be resolved, the reference in the input string will be unchanged. Double $$ are reduced to a single $, which allows for escaping the $(VAR_NAME) syntax: i.e. "$$(VAR_NAME)" will produce the string literal "$(VAR_NAME)". Escaped references will never be expanded, regardless of whether the variable exists or not. Defaults to "".' - type: string - valueFrom: - description: Source for the environment variable's value. Cannot be used if value is not empty. - properties: - configMapKeyRef: - description: Selects a key of a ConfigMap. - properties: - key: - description: The key to select. - type: string - name: - description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?' - type: string - optional: - description: Specify whether the ConfigMap or its key must be defined - type: boolean - required: - - key - type: object - fieldRef: - description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, `metadata.annotations['''']`, spec.nodeName, spec.serviceAccountName, status.hostIP, status.podIP, status.podIPs.' - properties: - apiVersion: - description: Version of the schema the FieldPath is written in terms of, defaults to "v1". - type: string - fieldPath: - description: Path of the field to select in the specified API version. - type: string - required: - - fieldPath - type: object - resourceFieldRef: - description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, limits.ephemeral-storage, requests.cpu, requests.memory and requests.ephemeral-storage) are currently supported.' - properties: - containerName: - description: 'Container name: required for volumes, optional for env vars' - type: string - divisor: - anyOf: - - type: integer - - type: string - description: Specifies the output format of the exposed resources, defaults to "1" - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - resource: - description: 'Required: resource to select' - type: string - required: - - resource - type: object - secretKeyRef: - description: Selects a key of a secret in the pod's namespace - properties: - key: - description: The key of the secret to select from. Must be a valid secret key. - type: string - name: - description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?' - type: string - optional: - description: Specify whether the Secret or its key must be defined - type: boolean - required: - - key - type: object - type: object - required: - - name - type: object - type: array - volumeSize: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - type: object type: object status: description: QuayRegistryStatus defines the observed state of QuayRegistry. diff --git a/config/crd/bases/quay.redhat.com_quayregistries.yaml b/config/crd/bases/quay.redhat.com_quayregistries.yaml index e2553ebcb..d26d00a7f 100644 --- a/config/crd/bases/quay.redhat.com_quayregistries.yaml +++ b/config/crd/bases/quay.redhat.com_quayregistries.yaml @@ -140,95 +140,6 @@ spec: configBundleSecret: description: ConfigBundleSecret is the name of the Kubernetes `Secret` in the same namespace which contains the base Quay config and extra certs. type: string - overrides: - description: Overrides holds overrides for the Base component (quay-app). - properties: - env: - items: - description: EnvVar represents an environment variable present in a Container. - properties: - name: - description: Name of the environment variable. Must be a C_IDENTIFIER. - type: string - value: - description: 'Variable references $(VAR_NAME) are expanded using the previously defined environment variables in the container and any service environment variables. If a variable cannot be resolved, the reference in the input string will be unchanged. Double $$ are reduced to a single $, which allows for escaping the $(VAR_NAME) syntax: i.e. "$$(VAR_NAME)" will produce the string literal "$(VAR_NAME)". Escaped references will never be expanded, regardless of whether the variable exists or not. Defaults to "".' - type: string - valueFrom: - description: Source for the environment variable's value. Cannot be used if value is not empty. - properties: - configMapKeyRef: - description: Selects a key of a ConfigMap. - properties: - key: - description: The key to select. - type: string - name: - description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?' - type: string - optional: - description: Specify whether the ConfigMap or its key must be defined - type: boolean - required: - - key - type: object - fieldRef: - description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, `metadata.annotations['''']`, spec.nodeName, spec.serviceAccountName, status.hostIP, status.podIP, status.podIPs.' - properties: - apiVersion: - description: Version of the schema the FieldPath is written in terms of, defaults to "v1". - type: string - fieldPath: - description: Path of the field to select in the specified API version. - type: string - required: - - fieldPath - type: object - resourceFieldRef: - description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, limits.ephemeral-storage, requests.cpu, requests.memory and requests.ephemeral-storage) are currently supported.' - properties: - containerName: - description: 'Container name: required for volumes, optional for env vars' - type: string - divisor: - anyOf: - - type: integer - - type: string - description: Specifies the output format of the exposed resources, defaults to "1" - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - resource: - description: 'Required: resource to select' - type: string - required: - - resource - type: object - secretKeyRef: - description: Selects a key of a secret in the pod's namespace - properties: - key: - description: The key of the secret to select from. Must be a valid secret key. - type: string - name: - description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?' - type: string - optional: - description: Specify whether the Secret or its key must be defined - type: boolean - required: - - key - type: object - type: object - required: - - name - type: object - type: array - volumeSize: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - type: object type: object status: description: QuayRegistryStatus defines the observed state of QuayRegistry. diff --git a/controllers/quay/quayregistry_controller.go b/controllers/quay/quayregistry_controller.go index 74284d717..ba3496a0c 100644 --- a/controllers/quay/quayregistry_controller.go +++ b/controllers/quay/quayregistry_controller.go @@ -50,7 +50,7 @@ import ( const ( upgradePollInterval = time.Second * 10 upgradePollTimeout = time.Second * 6000 - creationPollInterval = time.Second * 1 + creationPollInterval = time.Second * 2 creationPollTimeout = time.Second * 600 GrafanaDashboardConfigMapNameSuffix = "grafana-dashboard-quay" @@ -375,8 +375,7 @@ func (r *QuayRegistryReconciler) Reconcile(ctx context.Context, req ctrl.Request ) } - updatedQuay, err = v1.EnsureDefaultComponents(quayContext, updatedQuay) - if err != nil { + if err = v1.EnsureDefaultComponents(quayContext, updatedQuay); err != nil { log.Error(err, "could not ensure default `spec.components`") return r.Requeue, err } @@ -695,7 +694,11 @@ func (r *QuayRegistryReconciler) createOrUpdateObject( creationPollTimeout, func() (bool, error) { if err := r.Client.Create(ctx, obj); err != nil { - return false, nil + if errors.IsAlreadyExists(err) { + log.Info("immutable resource being deleted, retry") + return false, nil + } + return true, err } return true, nil }, diff --git a/controllers/quay/quayregistry_controller_test.go b/controllers/quay/quayregistry_controller_test.go index 8c0c0f9cb..0a0ac9bb6 100644 --- a/controllers/quay/quayregistry_controller_test.go +++ b/controllers/quay/quayregistry_controller_test.go @@ -37,9 +37,9 @@ func newQuayRegistry(name, namespace string) *v1.QuayRegistry { Spec: v1.QuayRegistrySpec{}, } // TODO: Test omitting components and marking some as disabled/unmanaged... - quay, _ = v1.EnsureDefaultComponents( + v1.EnsureDefaultComponents( &quaycontext.QuayRegistryContext{SupportsRoutes: false, SupportsObjectStorage: false, SupportsMonitoring: false}, - quay.DeepCopy(), + quay, ) return quay @@ -445,9 +445,10 @@ var _ = Describe("Reconciling a QuayRegistry", func() { Expect(k8sClient.Get(context.Background(), quayRegistryName, &updatedQuayRegistry)).Should(Succeed()) - expectedQuay, err := v1.EnsureDefaultComponents( + expectedQuay := quayRegistry.DeepCopy() + err := v1.EnsureDefaultComponents( &quaycontext.QuayRegistryContext{SupportsRoutes: false, SupportsObjectStorage: false, SupportsMonitoring: false}, - quayRegistry.DeepCopy(), + expectedQuay, ) Expect(err).NotTo(HaveOccurred()) diff --git a/e2e/base_component/00-assert.yaml b/e2e/base_component/00-assert.yaml new file mode 100644 index 000000000..88f44b212 --- /dev/null +++ b/e2e/base_component/00-assert.yaml @@ -0,0 +1,26 @@ +apiVersion: quay.redhat.com/v1 +kind: QuayRegistry +metadata: + name: skynet +spec: + components: + - kind: base + managed: true + - kind: postgres + managed: true + - kind: clair + managed: true + - kind: redis + managed: true + - kind: horizontalpodautoscaler + managed: true + - kind: objectstorage + managed: true + - kind: route + managed: true + - kind: mirror + managed: true + - kind: monitoring + managed: true + - kind: tls + managed: true diff --git a/e2e/base_component/00-create-quay-registry.yaml b/e2e/base_component/00-create-quay-registry.yaml new file mode 100644 index 000000000..7575d353a --- /dev/null +++ b/e2e/base_component/00-create-quay-registry.yaml @@ -0,0 +1,26 @@ +apiVersion: quay.redhat.com/v1 +kind: QuayRegistry +metadata: + name: skynet +spec: + components: + - kind: base + managed: false + - kind: postgres + managed: true + - kind: clair + managed: true + - kind: redis + managed: true + - kind: horizontalpodautoscaler + managed: true + - kind: objectstorage + managed: true + - kind: route + managed: true + - kind: mirror + managed: true + - kind: monitoring + managed: true + - kind: tls + managed: true diff --git a/e2e/environment_vars/00-assert.yaml b/e2e/environment_vars/00-assert.yaml new file mode 100644 index 000000000..83765d088 --- /dev/null +++ b/e2e/environment_vars/00-assert.yaml @@ -0,0 +1,86 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: skynet-quay-app +spec: + template: + spec: + containers: + - env: + - name: QE_K8S_CONFIG_SECRET + - name: QE_K8S_NAMESPACE + - name: DEBUGLOG + - name: WORKER_COUNT_WEB + - name: WORKER_COUNT_SECSCAN + - name: WORKER_COUNT_REGISTRY + - name: HTTP_PROXY + - name: HTTPS_PROXY + - name: NO_PROXY + - name: TESTING + value: TESTING +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: skynet-quay-database +spec: + template: + spec: + containers: + - env: + - name: POSTGRESQL_USER + - name: POSTGRESQL_DATABASE + - name: POSTGRESQL_ADMIN_PASSWORD + - name: POSTGRESQL_PASSWORD + - name: POSTGRESQL_SHARED_BUFFERS + - name: POSTGRESQL_MAX_CONNECTIONS + - name: TESTING + value: TESTING +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: skynet-clair-app +spec: + template: + spec: + containers: + - env: + - name: CLAIR_CONF + - name: CLAIR_MODE + - name: HTTP_PROXY + - name: HTTPS_PROXY + - name: NO_PROXY + - name: TESTING + value: TESTING +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: skynet-quay-redis +spec: + template: + spec: + containers: + - env: + - name: TESTING + value: TESTING +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: skynet-quay-mirror +spec: + template: + spec: + containers: + - env: + - name: QE_K8S_CONFIG_SECRET + - name: QE_K8S_NAMESPACE + - name: DEBUGLOG + - name: ENSURE_NO_MIGRATION + - name: HTTP_PROXY + - name: HTTPS_PROXY + - name: NO_PROXY + - name: TESTING + value: TESTING diff --git a/e2e/environment_vars/00-create-quay-registry.yaml b/e2e/environment_vars/00-create-quay-registry.yaml new file mode 100644 index 000000000..b2358f470 --- /dev/null +++ b/e2e/environment_vars/00-create-quay-registry.yaml @@ -0,0 +1,36 @@ +apiVersion: quay.redhat.com/v1 +kind: QuayRegistry +metadata: + name: skynet +spec: + components: + - kind: base + managed: true + overrides: + env: + - name: TESTING + value: TESTING + - kind: postgres + managed: true + overrides: + env: + - name: TESTING + value: TESTING + - kind: clair + managed: true + overrides: + env: + - name: TESTING + value: TESTING + - kind: redis + managed: true + overrides: + env: + - name: TESTING + value: TESTING + - kind: mirror + managed: true + overrides: + env: + - name: TESTING + value: TESTING diff --git a/e2e/min_deployment/00-assert.yaml b/e2e/min_deployment/00-assert.yaml index 5c5a2792e..90debcf90 100644 --- a/e2e/min_deployment/00-assert.yaml +++ b/e2e/min_deployment/00-assert.yaml @@ -10,6 +10,8 @@ spec: managed: false - kind: clairpostgres managed: false + - kind: base + managed: true - kind: postgres managed: true - kind: redis diff --git a/pkg/kustomize/kustomize.go b/pkg/kustomize/kustomize.go index afa42a7f2..9e0407a61 100644 --- a/pkg/kustomize/kustomize.go +++ b/pkg/kustomize/kustomize.go @@ -387,7 +387,7 @@ func KustomizationFor( } for _, component := range quay.Spec.Components { - if !component.Managed { + if !component.Managed || component.Kind == v1.ComponentBase { continue } diff --git a/pkg/kustomize/secrets.go b/pkg/kustomize/secrets.go index 7235ee824..047a13460 100644 --- a/pkg/kustomize/secrets.go +++ b/pkg/kustomize/secrets.go @@ -148,6 +148,9 @@ func FieldGroupFor( case v1.ComponentClairPostgres: return nil, nil + case v1.ComponentBase: + return nil, nil + default: return nil, fmt.Errorf("unknown component: %s", component) } @@ -232,8 +235,10 @@ func ContainsComponentConfig( switch component.Kind { case v1.ComponentClair: fields = (&securityscanner.SecurityScannerFieldGroup{}).Fields() + case v1.ComponentPostgres: fields = (&database.DatabaseFieldGroup{}).Fields() + case v1.ComponentClairPostgres: clairConfigBytes, ok := configBundle["clair-config.yaml"] // Clair config not provided @@ -254,23 +259,33 @@ func ContainsComponentConfig( case v1.ComponentRedis: fields = (&redis.RedisFieldGroup{}).Fields() + case v1.ComponentObjectStorage: fields = (&distributedstorage.DistributedStorageFieldGroup{}).Fields() + case v1.ComponentHPA: // HorizontalPodAutoscaler has no associated config fieldgroup. return false, nil + case v1.ComponentMirror: fields = (&repomirror.RepoMirrorFieldGroup{}).Fields() + case v1.ComponentRoute: fields = (&hostsettings.HostSettingsFieldGroup{}).Fields() + case v1.ComponentMonitoring: return false, nil + case v1.ComponentTLS: _, keyPresent := configBundle["ssl.key"] _, certPresent := configBundle["ssl.cert"] if certPresent && keyPresent { return true, nil } + + case v1.ComponentBase: + return false, nil + default: return false, fmt.Errorf("unknown component: %s", component.Kind) } diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index 837c92055..dab7ebe61 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -240,13 +240,6 @@ func FlattenSecret(configBundle *corev1.Secret) (*corev1.Secret, error) { // not Base. Base is not consider an ordinary component so its env overides are kept in the // root of the spec object. func getEnvOverrideForComponent(quay *v1.QuayRegistry, kind v1.ComponentKind) []corev1.EnvVar { - if kind == v1.ComponentBase { - if quay.Spec.Overrides == nil { - return nil - } - return quay.Spec.Overrides.Env - } - for _, cmp := range quay.Spec.Components { if cmp.Kind != kind { continue