From 3cca0357149bac44ce468554bee3d536a1f9543c Mon Sep 17 00:00:00 2001 From: OpenShift Cherrypick Robot Date: Fri, 11 Oct 2024 19:30:22 +0200 Subject: [PATCH] [redhat-3.13] pgmigration: fixing managed clair postgres migration error (PROJQUAY-8048) (#976) Clair and Quay postgres migrations were controlled by a single env variable. It was checking that the postgres version of new clair-postgres pods i.e postgres-15 was not matching the expected postgres version i.e. postgres-13. This retriggered the upgrade process making the postgres upgrade job stuck in the loop. This change separates the clair postgres migration component and rectify the logic of validating the upgrade job completion. --------- Co-authored-by: Shubhra Deshpande --- apis/quay/v1/quayregistry_types.go | 2 + .../quay-operator.clusterserviceversion.yaml | 4 ++ controllers/quay/features.go | 58 +++++++++++-------- hack/build.sh | 6 ++ hack/prepare-upstream.sh | 6 ++ .../clair-pg-scale-up.patch.yaml | 6 -- .../clair.deployment-scale-up.patch.yaml | 6 -- .../scale-down-clair/kustomization.yaml | 4 +- .../{postgres.go => quaypostgres.go} | 0 ...{postgres_test.go => quaypostgres_test.go} | 0 pkg/kustomize/kustomize.go | 55 ++++++++++++++---- pkg/kustomize/kustomize_test.go | 14 +++-- pkg/middleware/middleware.go | 2 +- pkg/middleware/middleware_test.go | 1 + 14 files changed, 107 insertions(+), 57 deletions(-) delete mode 100644 kustomize/components/clairpgupgrade/scale-down-clair/clair-pg-scale-up.patch.yaml delete mode 100644 kustomize/components/clairpgupgrade/scale-down-clair/clair.deployment-scale-up.patch.yaml rename pkg/cmpstatus/{postgres.go => quaypostgres.go} (100%) rename pkg/cmpstatus/{postgres_test.go => quaypostgres_test.go} (100%) diff --git a/apis/quay/v1/quayregistry_types.go b/apis/quay/v1/quayregistry_types.go index 74ffb4b04..5ccd568f3 100644 --- a/apis/quay/v1/quayregistry_types.go +++ b/apis/quay/v1/quayregistry_types.go @@ -83,6 +83,7 @@ var requiredComponents = []ComponentKind{ var supportsVolumeOverride = []ComponentKind{ ComponentPostgres, ComponentClair, + ComponentClairPostgres, } var supportsEnvOverride = []ComponentKind{ @@ -91,6 +92,7 @@ var supportsEnvOverride = []ComponentKind{ ComponentMirror, ComponentPostgres, ComponentRedis, + ComponentClairPostgres, } var supportsResourceOverrides = []ComponentKind{ diff --git a/bundle/manifests/quay-operator.clusterserviceversion.yaml b/bundle/manifests/quay-operator.clusterserviceversion.yaml index 6c4700a61..2c45bf758 100644 --- a/bundle/manifests/quay-operator.clusterserviceversion.yaml +++ b/bundle/manifests/quay-operator.clusterserviceversion.yaml @@ -161,6 +161,10 @@ spec: value: quay.io/sclorg/postgresql-13-c9s:latest - name: RELATED_IMAGE_COMPONENT_POSTGRES_PREVIOUS value: centos/postgresql-10-centos7:latest + - name: RELATED_IMAGE_COMPONENT_CLAIRPOSTGRES + value: quay.io/sclorg/postgresql-15-c9s:latest + - name: RELATED_IMAGE_COMPONENT_CLAIRPOSTGRES_PREVIOUS + value: quay.io/sclorg/postgresql-13-c9s:latest - name: RELATED_IMAGE_COMPONENT_REDIS value: docker.io/library/redis:7.0 serviceAccountName: quay-operator diff --git a/controllers/quay/features.go b/controllers/quay/features.go index b2694fb53..6bfadfc26 100644 --- a/controllers/quay/features.go +++ b/controllers/quay/features.go @@ -408,14 +408,20 @@ func (r *QuayRegistryReconciler) checkMonitoringAvailable( func (r *QuayRegistryReconciler) checkNeedsPostgresUpgradeForComponent( ctx context.Context, qctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, component v1.ComponentKind, ) error { - var deploymentName string - if component == v1.ComponentClairPostgres { - deploymentName = fmt.Sprintf("%s-%s", quay.GetName(), "clair-postgres") - } else if component == v1.ComponentPostgres { - deploymentName = fmt.Sprintf("%s-%s", quay.GetName(), "quay-database") - } else { + componentInfo := map[v1.ComponentKind]struct { + deploymentSuffix string + upgradeField *bool + }{ + v1.ComponentClairPostgres: {"clair-postgres", &qctx.NeedsClairPgUpgrade}, + v1.ComponentPostgres: {"quay-database", &qctx.NeedsPgUpgrade}, + } + + info, ok := componentInfo[component] + if !ok { return fmt.Errorf("invalid component kind: %s", component) } + + deploymentName := fmt.Sprintf("%s-%s", quay.GetName(), info.deploymentSuffix) r.Log.Info(fmt.Sprintf("getting %s version", component)) postgresDeployment := &appsv1.Deployment{} @@ -431,39 +437,41 @@ func (r *QuayRegistryReconciler) checkNeedsPostgresUpgradeForComponent( return nil } - r.Log.Info(fmt.Sprintf("%s deployment found", component), "image", postgresDeployment.Spec.Template.Spec.Containers[0].Image) deployedImageName := postgresDeployment.Spec.Template.Spec.Containers[0].Image - expectedImage, err := kustomize.ComponentImageFor(v1.ComponentPostgres) + r.Log.Info(fmt.Sprintf("%s deployment found", component), "image", deployedImageName) + + expectedImage, err := kustomize.ComponentImageFor(component) if err != nil { r.Log.Error(err, "failed to get postgres image") } - var expectedName string - if expectedImage.NewName != "" { - expectedName = expectedImage.NewName - } else { + expectedName := expectedImage.NewName + if expectedName == "" { expectedName = expectedImage.Name } - currentName := deployedImageName - if len(strings.Split(currentName, "@")) == 2 { - currentName = strings.Split(currentName, "@")[0] - } else if len(strings.Split(currentName, ":")) == 2 { - currentName = strings.Split(currentName, ":")[0] - } + + currentName := extractImageName(deployedImageName) + if currentName != expectedName { - if component == v1.ComponentClairPostgres { - r.Log.Info("clair-postgres needs to perform an upgrade, marking in context") - qctx.NeedsClairPgUpgrade = true - } else if component == v1.ComponentPostgres { - r.Log.Info("postgres needs to perform an upgrade, marking in context") - qctx.NeedsPgUpgrade = true - } + r.Log.Info(fmt.Sprintf("%s needs to perform an upgrade, marking in context", component)) + *info.upgradeField = true } else { r.Log.Info(fmt.Sprintf("%s does not need to perform an upgrade", component)) } return nil +} +func extractImageName(imageName string) string { + parts := strings.Split(imageName, "@") + if len(parts) > 1 { + return parts[0] + } + parts = strings.Split(imageName, ":") + if len(parts) > 1 { + return parts[0] + } + return imageName } // Taken from https://stackoverflow.com/questions/46735347/how-can-i-fetch-a-certificate-from-a-url diff --git a/hack/build.sh b/hack/build.sh index de8d773bc..81c3563fb 100755 --- a/hack/build.sh +++ b/hack/build.sh @@ -69,6 +69,8 @@ digest "${REGISTRY}/${NAMESPACE}/quay-builder:${TAG}" BUILDER_DIGEST digest "${REGISTRY}/${NAMESPACE}/quay-builder-qemu:3.9.0" BUILDER_QEMU_DIGEST digest quay.io/sclorg/postgresql-13-c9s:latest POSTGRES_DIGEST digest centos/postgresql-10-centos7:latest POSTGRES_OLD_DIGEST +digest quay.io/sclorg/postgresql-15-c9s:latest POSTGRES_CLAIR_DIGEST +digest quay.io/sclorg/postgresql-13-c9s:latest POSTGRES_CLAIR_OLD_DIGEST digest docker.io/library/redis:7.0 REDIS_DIGEST # need exporting so that yq can see them @@ -79,6 +81,8 @@ export BUILDER_DIGEST export BUILDER_QEMU_DIGEST export POSTGRES_DIGEST export POSTGRES_OLD_DIGEST +export POSTGRES_CLAIR_DIGEST +export POSTGRES_CLAIR_OLD_DIGEST export REDIS_DIGEST @@ -98,6 +102,8 @@ yq eval -i ' .spec.install.spec.deployments[0].spec.template.spec.containers[0].env[] |= select(.name == "RELATED_IMAGE_COMPONENT_BUILDER_QEMU") .value = strenv(BUILDER_QEMU_DIGEST) | .spec.install.spec.deployments[0].spec.template.spec.containers[0].env[] |= select(.name == "RELATED_IMAGE_COMPONENT_POSTGRES") .value = strenv(POSTGRES_DIGEST) | .spec.install.spec.deployments[0].spec.template.spec.containers[0].env[] |= select(.name == "RELATED_IMAGE_COMPONENT_POSTGRES_PREVIOUS") .value = strenv(POSTGRES_OLD_DIGEST) | + .spec.install.spec.deployments[0].spec.template.spec.containers[0].env[] |= select(.name == "RELATED_IMAGE_COMPONENT_CLAIRPOSTGRES") .value = strenv(POSTGRES_CLAIR_DIGEST) | + .spec.install.spec.deployments[0].spec.template.spec.containers[0].env[] |= select(.name == "RELATED_IMAGE_COMPONENT_CLAIRPOSTGRES_PREVIOUS") .value = strenv(POSTGRES_CLAIR_OLD_DIGEST) | .spec.install.spec.deployments[0].spec.template.spec.containers[0].env[] |= select(.name == "RELATED_IMAGE_COMPONENT_REDIS") .value = strenv(REDIS_DIGEST) ' "${CSV_PATH}" diff --git a/hack/prepare-upstream.sh b/hack/prepare-upstream.sh index d33fbe51c..84c172a3d 100755 --- a/hack/prepare-upstream.sh +++ b/hack/prepare-upstream.sh @@ -21,11 +21,15 @@ digest() { POSTGRES_DIGEST=$(digest POSTGRES) POSTGRES_PREVIOUS_DIGEST=$(digest POSTGRES_PREVIOUS) +POSTGRES_CLAIR_DIGEST=$(digest POSTGRES_CLAIR) +POSTGRES_CLAIR_PREVIOUS_DIGEST=$(digest POSTGRES_CLAIR_PREVIOUS) REDIS_DIGEST=$(digest REDIS) # export variables for yq export POSTGRES_DIGEST export POSTGRES_PREVIOUS_DIGEST +export POSTGRES_CLAIR_DIGEST +export POSTGRES_CLAIR_PREVIOUS_DIGEST export REDIS_DIGEST yq eval -i ' @@ -41,6 +45,8 @@ yq eval -i ' select(.name == "RELATED_IMAGE_COMPONENT_QUAY").value = ("quay.io/projectquay/quay:${RELEASE}" | envsubst) | select(.name == "RELATED_IMAGE_COMPONENT_POSTGRES").value = strenv(POSTGRES_DIGEST) | select(.name == "RELATED_IMAGE_COMPONENT_POSTGRES_PREVIOUS").value = strenv(POSTGRES_PREVIOUS_DIGEST) | + select(.name == "RELATED_IMAGE_COMPONENT_CLAIRPOSTGRES").value = strenv(POSTGRES_CLAIR_DIGEST) | + select(.name == "RELATED_IMAGE_COMPONENT_CLAIRPOSTGRES_PREVIOUS").value = strenv(POSTGRES_CLAIR_PREVIOUS_DIGEST) | select(.name == "RELATED_IMAGE_COMPONENT_REDIS").value = strenv(REDIS_DIGEST) ) | .spec.version = strenv(RELEASE) | diff --git a/kustomize/components/clairpgupgrade/scale-down-clair/clair-pg-scale-up.patch.yaml b/kustomize/components/clairpgupgrade/scale-down-clair/clair-pg-scale-up.patch.yaml deleted file mode 100644 index 875b1827f..000000000 --- a/kustomize/components/clairpgupgrade/scale-down-clair/clair-pg-scale-up.patch.yaml +++ /dev/null @@ -1,6 +0,0 @@ -apiVersion: apps/v1 -kind: Deployment -metadata: - name: clair-app -spec: - replicas: 2 diff --git a/kustomize/components/clairpgupgrade/scale-down-clair/clair.deployment-scale-up.patch.yaml b/kustomize/components/clairpgupgrade/scale-down-clair/clair.deployment-scale-up.patch.yaml deleted file mode 100644 index 25835358e..000000000 --- a/kustomize/components/clairpgupgrade/scale-down-clair/clair.deployment-scale-up.patch.yaml +++ /dev/null @@ -1,6 +0,0 @@ -apiVersion: apps/v1 -kind: Deployment -metadata: - name: clair-postgres -spec: - replicas: 2 \ No newline at end of file diff --git a/kustomize/components/clairpgupgrade/scale-down-clair/kustomization.yaml b/kustomize/components/clairpgupgrade/scale-down-clair/kustomization.yaml index 52e98776d..98ef6db54 100644 --- a/kustomize/components/clairpgupgrade/scale-down-clair/kustomization.yaml +++ b/kustomize/components/clairpgupgrade/scale-down-clair/kustomization.yaml @@ -2,7 +2,5 @@ apiVersion: kustomize.config.k8s.io/v1alpha1 kind: Component patchesStrategicMerge: - ./clair.deployment.patch.yaml - - ./clair-pg-scale-up.patch.yaml - - ./clair.deployment-scale-up.patch.yaml components: - - ../base + - "../base" \ No newline at end of file diff --git a/pkg/cmpstatus/postgres.go b/pkg/cmpstatus/quaypostgres.go similarity index 100% rename from pkg/cmpstatus/postgres.go rename to pkg/cmpstatus/quaypostgres.go diff --git a/pkg/cmpstatus/postgres_test.go b/pkg/cmpstatus/quaypostgres_test.go similarity index 100% rename from pkg/cmpstatus/postgres_test.go rename to pkg/cmpstatus/quaypostgres_test.go diff --git a/pkg/kustomize/kustomize.go b/pkg/kustomize/kustomize.go index b72e63ced..5ef100868 100644 --- a/pkg/kustomize/kustomize.go +++ b/pkg/kustomize/kustomize.go @@ -3,7 +3,6 @@ package kustomize import ( "errors" "fmt" - "os" "path/filepath" "runtime" @@ -52,16 +51,18 @@ const ( // to use. If set, returns a Kustomize image override for the given component. func ComponentImageFor(component v1.ComponentKind) (types.Image, error) { envVarFor := map[v1.ComponentKind]string{ - v1.ComponentQuay: componentImagePrefix + "QUAY", - v1.ComponentClair: componentImagePrefix + "CLAIR", - v1.ComponentRedis: componentImagePrefix + "REDIS", - v1.ComponentPostgres: componentImagePrefix + "POSTGRES", + v1.ComponentQuay: componentImagePrefix + "QUAY", + v1.ComponentClair: componentImagePrefix + "CLAIR", + v1.ComponentRedis: componentImagePrefix + "REDIS", + v1.ComponentPostgres: componentImagePrefix + "POSTGRES", + v1.ComponentClairPostgres: componentImagePrefix + "CLAIRPOSTGRES", } defaultImagesFor := map[v1.ComponentKind]string{ - v1.ComponentQuay: "quay.io/projectquay/quay", - v1.ComponentClair: "quay.io/projectquay/clair", - v1.ComponentRedis: "docker.io/library/redis", - v1.ComponentPostgres: "quay.io/sclorg/postgresql-13-c9s", + v1.ComponentQuay: "quay.io/projectquay/quay", + v1.ComponentClair: "quay.io/projectquay/clair", + v1.ComponentRedis: "docker.io/library/redis", + v1.ComponentPostgres: "quay.io/sclorg/postgresql-13-c9s", + v1.ComponentClairPostgres: "quay.io/sclorg/postgresql-15-c9s", } imageOverride := types.Image{ @@ -100,6 +101,31 @@ func postgresUpgradeImage() (types.Image, error) { return imageOverride, nil } + if len(strings.Split(image, "@")) == 2 { + imageOverride.NewName = strings.Split(image, "@")[0] + imageOverride.Digest = strings.Split(image, "@")[1] + } else if len(strings.Split(image, ":")) == 2 { + imageOverride.NewName = strings.Split(image, ":")[0] + imageOverride.NewTag = strings.Split(image, ":")[1] + } else { + return types.Image{}, fmt.Errorf( + "image override must be reference by tag or digest: %s", image, + ) + } + return imageOverride, nil +} + +func clairpostgresUpgradeImage() (types.Image, error) { + imageOverride := types.Image{ + Name: "quay.io/sclorg/postgresql-13-c9s", + } + + image := os.Getenv("RELATED_IMAGE_COMPONENT_CLAIRPOSTGRES_PREVIOUS") + + if image == "" { + return imageOverride, nil + } + if len(strings.Split(image, "@")) == 2 { imageOverride.NewName = strings.Split(image, "@")[0] imageOverride.Digest = strings.Split(image, "@")[1] @@ -442,6 +468,7 @@ func KustomizationFor( } else { componentPaths = append(componentPaths, "../components/clairpgupgrade/base") } + } images := []types.Image{} @@ -457,7 +484,7 @@ func KustomizationFor( } } - if ctx.NeedsPgUpgrade || ctx.NeedsClairPgUpgrade { + if ctx.NeedsPgUpgrade { pgImage, err := postgresUpgradeImage() if err != nil { return nil, err @@ -465,6 +492,14 @@ func KustomizationFor( images = append(images, pgImage) } + if ctx.NeedsClairPgUpgrade { + clairPgImage, err := clairpostgresUpgradeImage() + if err != nil { + return nil, err + } + images = append(images, clairPgImage) + } + return &types.Kustomization{ TypeMeta: types.TypeMeta{ APIVersion: types.KustomizationVersion, diff --git a/pkg/kustomize/kustomize_test.go b/pkg/kustomize/kustomize_test.go index 2ec37be44..2bbe57200 100644 --- a/pkg/kustomize/kustomize_test.go +++ b/pkg/kustomize/kustomize_test.go @@ -45,6 +45,7 @@ var kustomizationForTests = []struct { {Kind: "postgres", Managed: true}, {Kind: "clair", Managed: true}, {Kind: "redis", Managed: true}, + {Kind: "clairpostgres", Managed: true}, {Kind: "objectstorage", Managed: true}, {Kind: "mirror", Managed: true}, }, @@ -61,6 +62,7 @@ var kustomizationForTests = []struct { "../components/postgres", "../components/clair", "../components/redis", + "../components/clairpostgres", "../components/objectstorage", "../components/mirror", }, @@ -207,14 +209,14 @@ var kustomizationForTests = []struct { &v1.QuayRegistry{ Spec: v1.QuayRegistrySpec{ Components: []v1.Component{ - {Kind: "postgres", Managed: true}, + {Kind: "clairpostgres", Managed: true}, {Kind: "clair", Managed: false}, {Kind: "redis", Managed: true}, }, }, }, quaycontext.QuayRegistryContext{ - NeedsPgUpgrade: true, + NeedsClairPgUpgrade: true, }, &types.Kustomization{ TypeMeta: types.TypeMeta{ @@ -223,15 +225,15 @@ var kustomizationForTests = []struct { }, Components: []string{ "../components/redis", - "../components/postgres", - "../components/pgupgrade", + "../components/clairpostgres", + "../components/clairpgupgrade/base", }, Images: []types.Image{ {Name: "quay.io/projectquay/quay", NewName: "quay", NewTag: "latest"}, {Name: "quay.io/projectquay/clair", NewName: "clair", NewTag: "alpine"}, {Name: "docker.io/library/redis", NewName: "redis", NewTag: "buster"}, - {Name: "quay.io/sclorg/postgresql-13-c9s", NewName: "postgres", NewTag: "latest"}, - {Name: "centos/postgresql-10-centos7", NewName: "postgres_previous", NewTag: "latest"}, + {Name: "quay.io/sclorg/postgresql-15-c9s", NewName: "clairpostgres", NewTag: "latest"}, + {Name: "quay.io/sclorg/postgresql-13-c9s", NewName: "clairpostgres_previous", NewTag: "latest"}, }, SecretGenerator: []types.SecretArgs{}, }, diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index 488577d33..03528f54e 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -201,7 +201,7 @@ func Process(quay *v1.QuayRegistry, qctx *quaycontext.QuayRegistryContext, obj c case "postgres": override = v1.GetVolumeSizeOverrideForComponent(quay, v1.ComponentPostgres) case "clair-postgres": - override = v1.GetVolumeSizeOverrideForComponent(quay, v1.ComponentClair) + override = v1.GetVolumeSizeOverrideForComponent(quay, v1.ComponentClairPostgres) } // If override was not provided diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index db22698af..d9d60fc27 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -183,6 +183,7 @@ var processTests = []struct { {Kind: "route", Managed: true}, {Kind: "tls", Managed: true}, {Kind: "postgres", Managed: true, Overrides: &v1.Override{VolumeSize: parseResourceString("70Gi")}}, + {Kind: "clairpostgres", Managed: true, Overrides: &v1.Override{VolumeSize: parseResourceString("60Gi")}}, {Kind: "clair", Managed: true, Overrides: &v1.Override{VolumeSize: parseResourceString("60Gi")}}, }, },