[redhat-3.13] pgmigration: fixing managed clair postgres migration er…

…ror (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 <shubhrajayant+github.com>

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{

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

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

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}"
 

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) |

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"

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,14 +484,22 @@ func KustomizationFor(
 		}
 	}
 
-	if ctx.NeedsPgUpgrade || ctx.NeedsClairPgUpgrade {
+	if ctx.NeedsPgUpgrade {
 		pgImage, err := postgresUpgradeImage()
 		if err != nil {
 			return nil, err
 		}
 		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,

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{},
 		},

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

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")}},
 				},
 			},