Updates to support changes starting in pgAdmin 9.3

Changes in the flags used by pgAdmin's setup.py for user managment
start in pgAdmin 9.3.

Issue: PGO-2686

Author: tjmoore4

config/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml

@@ -1900,6 +1900,10 @@ spec:
                 description: MajorVersion represents the major version of the running
                   pgAdmin.
                 type: integer
+              minorVersion:
+                description: MinorVersion represents the minor version of the running
+                  pgAdmin.
+                type: string
               observedGeneration:
                 description: observedGeneration represents the .metadata.generation
                   on which the status was based.

internal/controller/standalone_pgadmin/users.go

@@ -79,40 +79,55 @@ func (r *PGAdminReconciler) reconcilePGAdminUsers(ctx context.Context, pgadmin *
 		return nil
 	}
 
-	// If the pgAdmin version is not in the status or the image SHA has changed, get
-	// the pgAdmin version and store it in the status.
-	var pgadminVersion int
-	if pgadmin.Status.MajorVersion == 0 || pgadmin.Status.ImageSHA != pgAdminImageSha {
-		pgadminVersion, err = r.reconcilePGAdminMajorVersion(ctx, podExecutor)
+	// If the pgAdmin major or minor version is not in the status or the image
+	// SHA has changed, get the pgAdmin version and store it in the status.
+	var pgadminMajorVersion int
+	if pgadmin.Status.MajorVersion == 0 || pgadmin.Status.MinorVersion == "" ||
+		pgadmin.Status.ImageSHA != pgAdminImageSha {
+
+		pgadminMinorVersion, err := r.reconcilePGAdminVersion(ctx, podExecutor)
 		if err != nil {
 			return err
 		}
-		pgadmin.Status.MajorVersion = pgadminVersion
+
+		// ensure minor version is valid before storing in status
+		parsedMinorVersion, err := strconv.ParseFloat(pgadminMinorVersion, 64)
+		if err != nil {
+			return err
+		}
+
+		// Note: "When converting a floating-point number to an integer, the
+		// fraction is discarded (truncation towards zero)."
+		// - https://go.dev/ref/spec#Conversions
+		pgadminMajorVersion = int(parsedMinorVersion)
+
+		pgadmin.Status.MinorVersion = pgadminMinorVersion
+		pgadmin.Status.MajorVersion = pgadminMajorVersion
 		pgadmin.Status.ImageSHA = pgAdminImageSha
 	} else {
-		pgadminVersion = pgadmin.Status.MajorVersion
+		pgadminMajorVersion = pgadmin.Status.MajorVersion
 	}
 
 	// If the pgAdmin version is not v8 or higher, return early as user management is
 	// only supported for pgAdmin v8 and higher.
-	if pgadminVersion < 8 {
+	if pgadminMajorVersion < 8 {
 		// If pgAdmin version is less than v8 and user management is being attempted,
 		// log a message clarifying that it is only supported for pgAdmin v8 and higher.
 		if len(pgadmin.Spec.Users) > 0 {
 			log.Info("User management is only supported for pgAdmin v8 and higher.",
-				"pgadminVersion", pgadminVersion)
+				"pgadminVersion", pgadminMajorVersion)
 		}
 		return err
 	}
 
 	return r.writePGAdminUsers(ctx, pgadmin, podExecutor)
 }
 
-// reconcilePGAdminMajorVersion execs into the pgAdmin pod and retrieves the pgAdmin major version
-func (r *PGAdminReconciler) reconcilePGAdminMajorVersion(ctx context.Context, exec Executor) (int, error) {
+// reconcilePGAdminVersion execs into the pgAdmin pod and retrieves the pgAdmin minor version
+func (r *PGAdminReconciler) reconcilePGAdminVersion(ctx context.Context, exec Executor) (string, error) {
 	script := fmt.Sprintf(`
 PGADMIN_DIR=%s
-cd $PGADMIN_DIR && python3 -c "import config; print(config.APP_RELEASE)"
+cd $PGADMIN_DIR && python3 -c "import config; print(config.APP_VERSION)"
 `, pgAdminDir)
 
 	var stdin, stdout, stderr bytes.Buffer
@@ -121,10 +136,10 @@ cd $PGADMIN_DIR && python3 -c "import config; print(config.APP_RELEASE)"
 		[]string{"bash", "-ceu", "--", script}...)
 
 	if err != nil {
-		return 0, err
+		return "", err
 	}
 
-	return strconv.Atoi(strings.TrimSpace(stdout.String()))
+	return strings.TrimSpace(stdout.String()), nil
 }
 
 // writePGAdminUsers takes the users in the pgAdmin spec and writes (adds or updates) their data
@@ -170,10 +185,23 @@ cd $PGADMIN_DIR
 	for _, user := range existingUsersArr {
 		existingUsersMap[user.Username] = user
 	}
+
+	var olderThan9_3 bool
+	versionFloat, err := strconv.ParseFloat(pgadmin.Status.MinorVersion, 32)
+	if err != nil {
+		return err
+	}
+	if versionFloat < 9.3 {
+		olderThan9_3 = true
+	}
+
 	intentUsers := []pgAdminUserForJson{}
 	for _, user := range pgadmin.Spec.Users {
 		var stdin, stdout, stderr bytes.Buffer
-		typeFlag := "--nonadmin"
+		typeFlag := "--role User"
+		if olderThan9_3 {
+			typeFlag = "--nonadmin"
+		}
 		isAdmin := false
 		if user.Role == "Administrator" {
 			typeFlag = "--admin"

internal/controller/standalone_pgadmin/users_test.go

@@ -110,15 +110,16 @@ func TestReconcilePGAdminUsers(t *testing.T) {
 			assert.Equal(t, namespace, pgadmin.Namespace)
 			assert.Equal(t, container, naming.ContainerPGAdmin)
 
-			// Simulate a v7 version of pgAdmin by setting stdout to "7" for
-			// podexec call in reconcilePGAdminMajorVersion
-			stdout.Write([]byte("7"))
+			// Simulate a v7.1 version of pgAdmin by setting stdout to "7.1"
+			// for podexec call in reconcilePGAdminVersion
+			_, _ = stdout.Write([]byte("7.1"))
 			return nil
 		}
 
 		assert.NilError(t, r.reconcilePGAdminUsers(ctx, pgadmin))
 		assert.Equal(t, calls, 1, "PodExec should be called once")
 		assert.Equal(t, pgadmin.Status.MajorVersion, 7)
+		assert.Equal(t, pgadmin.Status.MinorVersion, "7.1")
 		assert.Equal(t, pgadmin.Status.ImageSHA, "fakeSHA")
 	})
 
@@ -145,20 +146,58 @@ func TestReconcilePGAdminUsers(t *testing.T) {
 		) error {
 			calls++
 
-			// Simulate a v7 version of pgAdmin by setting stdout to "7" for
-			// podexec call in reconcilePGAdminMajorVersion
-			stdout.Write([]byte("7"))
+			// Simulate a v7.1 version of pgAdmin by setting stdout to "7.1"
+			// for podexec call in reconcilePGAdminVersion
+			_, _ = stdout.Write([]byte("7.1"))
 			return nil
 		}
 
 		assert.NilError(t, r.reconcilePGAdminUsers(ctx, pgadmin))
 		assert.Equal(t, calls, 1, "PodExec should be called once")
 		assert.Equal(t, pgadmin.Status.MajorVersion, 7)
+		assert.Equal(t, pgadmin.Status.MinorVersion, "7.1")
 		assert.Equal(t, pgadmin.Status.ImageSHA, "newFakeSHA")
 	})
+
+	t.Run("PodHealthyBadVersion", func(t *testing.T) {
+		pgadmin := pgadmin.DeepCopy()
+		pod := pod.DeepCopy()
+
+		pod.DeletionTimestamp = nil
+		pod.Status.ContainerStatuses =
+			[]corev1.ContainerStatus{{Name: naming.ContainerPGAdmin}}
+		pod.Status.ContainerStatuses[0].State.Running =
+			new(corev1.ContainerStateRunning)
+		pod.Status.ContainerStatuses[0].ImageID = "fakeSHA"
+
+		r := new(PGAdminReconciler)
+		r.Client = fake.NewClientBuilder().WithObjects(pod).Build()
+
+		calls := 0
+		r.PodExec = func(
+			ctx context.Context, namespace, pod, container string,
+			stdin io.Reader, stdout, stderr io.Writer, command ...string,
+		) error {
+			calls++
+
+			assert.Equal(t, pod, "pgadmin-123-0")
+			assert.Equal(t, namespace, pgadmin.Namespace)
+			assert.Equal(t, container, naming.ContainerPGAdmin)
+
+			// set expected version to something completely wrong
+			_, _ = stdout.Write([]byte("woot"))
+			return nil
+		}
+
+		assert.ErrorContains(t, r.reconcilePGAdminUsers(ctx, pgadmin), "strconv.ParseFloat: parsing \"woot\": invalid syntax")
+		assert.Equal(t, calls, 1, "PodExec should be called once")
+		assert.Equal(t, pgadmin.Status.MajorVersion, 0)
+		assert.Equal(t, pgadmin.Status.MinorVersion, "")
+		assert.Equal(t, pgadmin.Status.ImageSHA, "")
+	})
 }
 
-func TestReconcilePGAdminMajorVersion(t *testing.T) {
+func TestReconcilePGAdminVersion(t *testing.T) {
 	ctx := context.Background()
 	pod := corev1.Pod{}
 	pod.Namespace = "test-namespace"
@@ -180,30 +219,15 @@ func TestReconcilePGAdminMajorVersion(t *testing.T) {
 			assert.Equal(t, namespace, "test-namespace")
 			assert.Equal(t, container, naming.ContainerPGAdmin)
 
-			// Simulate a v7 version of pgAdmin by setting stdout to "7" for
-			// podexec call in reconcilePGAdminMajorVersion
-			stdout.Write([]byte("7"))
+			// Simulate a v9.3 version of pgAdmin by setting stdout to "9.3"
+			// for podexec call in reconcilePGAdminVersion
+			_, _ = stdout.Write([]byte("9.3"))
 			return nil
 		}
 
-		version, err := reconciler.reconcilePGAdminMajorVersion(ctx, podExecutor)
+		version, err := reconciler.reconcilePGAdminVersion(ctx, podExecutor)
 		assert.NilError(t, err)
-		assert.Equal(t, version, 7)
-	})
-
-	t.Run("FailedRetrieval", func(t *testing.T) {
-		reconciler.PodExec = func(
-			ctx context.Context, namespace, pod, container string,
-			stdin io.Reader, stdout, stderr io.Writer, command ...string,
-		) error {
-			// Simulate the python call giving bad data (not a version int)
-			stdout.Write([]byte("asdfjkl;"))
-			return nil
-		}
-
-		version, err := reconciler.reconcilePGAdminMajorVersion(ctx, podExecutor)
-		assert.Check(t, err != nil)
-		assert.Equal(t, version, 0)
+		assert.Equal(t, version, "9.3")
 	})
 
 	t.Run("PodExecError", func(t *testing.T) {
@@ -214,9 +238,9 @@ func TestReconcilePGAdminMajorVersion(t *testing.T) {
 			return errors.New("PodExecError")
 		}
 
-		version, err := reconciler.reconcilePGAdminMajorVersion(ctx, podExecutor)
+		version, err := reconciler.reconcilePGAdminVersion(ctx, podExecutor)
 		assert.Check(t, err != nil)
-		assert.Equal(t, version, 0)
+		assert.Equal(t, version, "")
 	})
 }
 
@@ -238,6 +262,14 @@ func TestWritePGAdminUsers(t *testing.T) {
 	pgadmin.Namespace = ns.Name
 	assert.NilError(t, cc.Create(ctx, pgadmin))
 
+	// fake the status so that the correct commands will be used when creating
+	// users.
+	pgadmin.Status = v1beta1.PGAdminStatus{
+		ImageSHA:     "fakesha",
+		MajorVersion: 9,
+		MinorVersion: "9.3",
+	}
+
 	userPasswordSecret1 := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "user-password-secret1",

pkg/apis/postgres-operator.crunchydata.com/v1beta1/standalone_pgadmin_types.go

@@ -175,6 +175,10 @@ type PGAdminStatus struct {
 	// +optional
 	MajorVersion int `json:"majorVersion,omitempty"`
 
+	// MinorVersion represents the minor version of the running pgAdmin.
+	// +optional
+	MinorVersion string `json:"minorVersion,omitempty"`
+
 	// observedGeneration represents the .metadata.generation on which the status was based.
 	// +optional
 	// +kubebuilder:validation:Minimum=0