From 1de9a8b4c8bb836cf54cd39f3b7c2495bfbfed90 Mon Sep 17 00:00:00 2001 From: deshpandevlab Date: Fri, 11 Oct 2024 13:22:20 -0400 Subject: [PATCH] pgmigration: fixing managed clair postgres migration error (PROJQUAY-8048) (#975) 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 ++ .../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 + 12 files changed, 107 insertions(+), 45 deletions(-) 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/kustomization.yaml b/kustomize/components/clairpgupgrade/scale-down-clair/kustomization.yaml index 52e98776d..fa5c5fcba 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" 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")}}, }, },