From bce928f6eb644e3189b24cda9c4297d0a233b046 Mon Sep 17 00:00:00 2001 From: TJ Moore Date: Wed, 25 Oct 2023 12:40:41 -0400 Subject: [PATCH 1/2] Require a Postgres version when creating a PostgresCluster This change adds a new flag 'pg-major-version' to the 'create postgrescluster' command to allow a specific Postgres version to be used for the PostgresCluster. The new flag is required and does not default to a particular version. As before, the image used by the cluster will be pulled from the corresponding 'RELATED_IMAGE' value set in the environment. KUTTL test updates are also included, along with a bump to Postgres 15 and a timeout adjustment. Issue: PGO-446 --- .../reference/pgo_create_postgrescluster.md | 7 +-- internal/cmd/create.go | 17 ++++--- internal/cmd/create_test.go | 4 +- .../kuttl/e2e/create/00--create_cluster.yaml | 2 +- testing/kuttl/e2e/create/00-assert.yaml | 2 +- .../create/01--check_version_validation.yaml | 46 +++++++++++++++++++ .../kuttl/e2e/delete-cluster/00--cluster.yaml | 2 +- testing/kuttl/e2e/restore/00--cluster.yaml | 2 +- .../support-export/00--create_cluster.yaml | 2 +- .../kuttl/e2e/support-export/00-assert.yaml | 2 +- .../30--create_cluster_with_monitoring.yaml | 2 +- .../kuttl/e2e/support-export/30-assert.yaml | 2 +- testing/kuttl/kuttl-test.yaml | 2 +- 13 files changed, 72 insertions(+), 20 deletions(-) create mode 100644 testing/kuttl/e2e/create/01--check_version_validation.yaml diff --git a/docs/content/reference/pgo_create_postgrescluster.md b/docs/content/reference/pgo_create_postgrescluster.md index aac38782..4c0a55ec 100644 --- a/docs/content/reference/pgo_create_postgrescluster.md +++ b/docs/content/reference/pgo_create_postgrescluster.md @@ -23,8 +23,8 @@ pgo create postgrescluster CLUSTER_NAME [flags] ### Examples ``` -# Create a postgrescluster -pgo create postgrescluster hippo +# Create a postgrescluster with Postgres 15 +pgo create postgrescluster hippo --pg-major-version 15 ``` ### Example output @@ -35,7 +35,8 @@ postgresclusters/hippo created ### Options ``` - -h, --help help for postgrescluster + -h, --help help for postgrescluster + --pg-major-version int Set the Postgres major version ``` ### Options inherited from parent commands diff --git a/internal/cmd/create.go b/internal/cmd/create.go index f9a414da..9d8d73e2 100644 --- a/internal/cmd/create.go +++ b/internal/cmd/create.go @@ -17,6 +17,7 @@ package cmd import ( "context" "fmt" + "strconv" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -61,8 +62,12 @@ func newCreateClusterCommand(config *internal.Config) *cobra.Command { cmd.Args = cobra.ExactArgs(1) - cmd.Example = internal.FormatExample(`# Create a postgrescluster -pgo create postgrescluster hippo + var pgMajorVersion int + cmd.Flags().IntVar(&pgMajorVersion, "pg-major-version", 0, "Set the Postgres major version") + cobra.CheckErr(cmd.MarkFlagRequired("pg-major-version")) + + cmd.Example = internal.FormatExample(`# Create a postgrescluster with Postgres 15 +pgo create postgrescluster hippo --pg-major-version 15 ### Example output postgresclusters/hippo created`) @@ -82,7 +87,7 @@ postgresclusters/hippo created`) return err } - cluster, err := generateUnstructuredClusterYaml(clusterName) + cluster, err := generateUnstructuredClusterYaml(clusterName, strconv.Itoa(pgMajorVersion)) if err != nil { return err } @@ -104,7 +109,7 @@ postgresclusters/hippo created`) // generateUnstructuredClusterYaml takes a name and returns a PostgresCluster // in the unstructured format. -func generateUnstructuredClusterYaml(name string) (*unstructured.Unstructured, error) { +func generateUnstructuredClusterYaml(name, pgMajorVersion string) (*unstructured.Unstructured, error) { var cluster unstructured.Unstructured err := yaml.Unmarshal([]byte(fmt.Sprintf(` apiVersion: postgres-operator.crunchydata.com/v1beta1 @@ -112,7 +117,7 @@ kind: PostgresCluster metadata: name: %s spec: - postgresVersion: 14 + postgresVersion: %s instances: - dataVolumeClaimSpec: accessModes: @@ -131,7 +136,7 @@ spec: resources: requests: storage: 1Gi -`, name)), &cluster) +`, name, pgMajorVersion)), &cluster) if err != nil { return nil, err diff --git a/internal/cmd/create_test.go b/internal/cmd/create_test.go index 593ab703..d8c74911 100644 --- a/internal/cmd/create_test.go +++ b/internal/cmd/create_test.go @@ -47,10 +47,10 @@ spec: resources: requests: storage: 1Gi - postgresVersion: 14 + postgresVersion: 15 ` - u, err := generateUnstructuredClusterYaml("hippo") + u, err := generateUnstructuredClusterYaml("hippo", "15") assert.NilError(t, err) assert.Assert(t, cmp.MarshalMatches( diff --git a/testing/kuttl/e2e/create/00--create_cluster.yaml b/testing/kuttl/e2e/create/00--create_cluster.yaml index 392e894e..c3420348 100644 --- a/testing/kuttl/e2e/create/00--create_cluster.yaml +++ b/testing/kuttl/e2e/create/00--create_cluster.yaml @@ -2,4 +2,4 @@ apiVersion: kuttl.dev/v1beta1 kind: TestStep commands: -- script: kubectl-pgo --namespace $NAMESPACE create postgrescluster kuttl-create-cluster +- script: kubectl-pgo --namespace $NAMESPACE create postgrescluster --pg-major-version 15 kuttl-create-cluster diff --git a/testing/kuttl/e2e/create/00-assert.yaml b/testing/kuttl/e2e/create/00-assert.yaml index 40b260c0..f3fe2512 100644 --- a/testing/kuttl/e2e/create/00-assert.yaml +++ b/testing/kuttl/e2e/create/00-assert.yaml @@ -22,7 +22,7 @@ spec: requests: storage: 1Gi replicas: 1 - postgresVersion: 14 + postgresVersion: 15 status: instances: - name: "00" diff --git a/testing/kuttl/e2e/create/01--check_version_validation.yaml b/testing/kuttl/e2e/create/01--check_version_validation.yaml new file mode 100644 index 00000000..44fac127 --- /dev/null +++ b/testing/kuttl/e2e/create/01--check_version_validation.yaml @@ -0,0 +1,46 @@ +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: | + # Verify the error when the Postgres version given is too low. + TOO_LOW=$(kubectl-pgo create postgrescluster --pg-major-version=1 toolow 2>&1) + if [[ "${TOO_LOW}" != "Error:"*"Invalid value"* ]]; then + printf 'Expected invalid value error, got %q\n' "${TOO_LOW}" + exit 1 + fi + + # Verify the error when the Postgres version given is too high. + TOO_HIGH=$(kubectl-pgo create postgrescluster --pg-major-version=100 toohigh 2>&1) + if [[ "${TOO_HIGH}" != "Error:"*"Invalid value"* ]]; then + printf 'Expected invalid value error, got %q\n' "${TOO_HIGH}" + exit 1 + fi + + # Verify the error when the Postgres version is not an integer. + NOT_INT=$(kubectl-pgo create postgrescluster --pg-major-version=15.1 notint 2>&1) + if [[ "${NOT_INT}" != "Error: invalid argument"* ]]; then + printf 'Expected invalid argument error, got %q\n' "${NOT_INT}" + exit 1 + fi + + # Verify the error when the Postgres version is not a number. + NOT_NUM=$(kubectl-pgo create postgrescluster --pg-major-version=x notnum 2>&1) + if [[ "${NOT_NUM}" != "Error: invalid argument"* ]]; then + printf 'Expected invalid argument error, got %q\n' "${NOT_NUM}" + exit 1 + fi + + # Verify the error when the Postgres version flag is not provided. + MISSING=$(kubectl-pgo create postgrescluster missing 2>&1) + if [[ "${MISSING}" != "Error: required flag"* ]]; then + printf 'Expected required flag error, got %q\n' "${MISSING}" + exit 1 + fi + + # Verify the error when the Postgres version value is empty. + NOT_SET=$(kubectl-pgo create postgrescluster --pg-major-version= notset 2>&1) + if [[ "${NOT_SET}" != "Error: invalid argument"* ]]; then + printf 'Expected invalid argument error, got %q\n' "${NOT_SET}" + exit 1 + fi diff --git a/testing/kuttl/e2e/delete-cluster/00--cluster.yaml b/testing/kuttl/e2e/delete-cluster/00--cluster.yaml index 6b6c7edd..7dc158e3 100644 --- a/testing/kuttl/e2e/delete-cluster/00--cluster.yaml +++ b/testing/kuttl/e2e/delete-cluster/00--cluster.yaml @@ -3,7 +3,7 @@ kind: PostgresCluster metadata: name: delete-cluster spec: - postgresVersion: 14 + postgresVersion: 15 instances: - name: instance1 dataVolumeClaimSpec: diff --git a/testing/kuttl/e2e/restore/00--cluster.yaml b/testing/kuttl/e2e/restore/00--cluster.yaml index 873d8cd8..bff771f7 100644 --- a/testing/kuttl/e2e/restore/00--cluster.yaml +++ b/testing/kuttl/e2e/restore/00--cluster.yaml @@ -4,7 +4,7 @@ kind: PostgresCluster metadata: name: restore-cluster spec: - postgresVersion: 14 + postgresVersion: 15 instances: - name: instance1 dataVolumeClaimSpec: diff --git a/testing/kuttl/e2e/support-export/00--create_cluster.yaml b/testing/kuttl/e2e/support-export/00--create_cluster.yaml index 327255f7..10b2f33d 100644 --- a/testing/kuttl/e2e/support-export/00--create_cluster.yaml +++ b/testing/kuttl/e2e/support-export/00--create_cluster.yaml @@ -2,4 +2,4 @@ apiVersion: kuttl.dev/v1beta1 kind: TestStep commands: -- script: kubectl-pgo --namespace $NAMESPACE create postgrescluster kuttl-support-cluster +- script: kubectl-pgo --namespace $NAMESPACE create postgrescluster --pg-major-version 15 kuttl-support-cluster diff --git a/testing/kuttl/e2e/support-export/00-assert.yaml b/testing/kuttl/e2e/support-export/00-assert.yaml index a0c3f91c..b935a0f3 100644 --- a/testing/kuttl/e2e/support-export/00-assert.yaml +++ b/testing/kuttl/e2e/support-export/00-assert.yaml @@ -22,7 +22,7 @@ spec: requests: storage: 1Gi replicas: 1 - postgresVersion: 14 + postgresVersion: 15 status: instances: - name: "00" diff --git a/testing/kuttl/e2e/support-export/30--create_cluster_with_monitoring.yaml b/testing/kuttl/e2e/support-export/30--create_cluster_with_monitoring.yaml index d16b9b2d..dbab9892 100644 --- a/testing/kuttl/e2e/support-export/30--create_cluster_with_monitoring.yaml +++ b/testing/kuttl/e2e/support-export/30--create_cluster_with_monitoring.yaml @@ -3,7 +3,7 @@ kind: PostgresCluster metadata: name: kuttl-support-monitoring-cluster spec: - postgresVersion: 14 + postgresVersion: 15 instances: - dataVolumeClaimSpec: accessModes: [ReadWriteOnce] diff --git a/testing/kuttl/e2e/support-export/30-assert.yaml b/testing/kuttl/e2e/support-export/30-assert.yaml index 83f1f5fd..771e07f3 100644 --- a/testing/kuttl/e2e/support-export/30-assert.yaml +++ b/testing/kuttl/e2e/support-export/30-assert.yaml @@ -22,7 +22,7 @@ spec: requests: storage: 1Gi replicas: 1 - postgresVersion: 14 + postgresVersion: 15 status: instances: - name: "00" diff --git a/testing/kuttl/kuttl-test.yaml b/testing/kuttl/kuttl-test.yaml index 6e85360d..7d923e87 100644 --- a/testing/kuttl/kuttl-test.yaml +++ b/testing/kuttl/kuttl-test.yaml @@ -2,7 +2,7 @@ apiVersion: kuttl.dev/v1beta1 kind: TestSuite testDirs: - testing/kuttl/e2e/ -timeout: 180 +timeout: 300 parallel: 2 # by default kuttl will run in a generated namespace to override # that functionality simply uncomment the line below and replace From 35b01ba90aa34b765312b1462cd1a687a7f20bc6 Mon Sep 17 00:00:00 2001 From: TJ Moore Date: Wed, 25 Oct 2023 12:50:25 -0400 Subject: [PATCH 2/2] Update 'backup' KUTTL test Improve error printing behavior and ensure a backup annotation race condition won't happen in faster test environments. --- testing/kuttl/e2e/backup/08--backup-with-just-trigger.yaml | 6 ++++-- testing/kuttl/e2e/backup/11--backup-with-new-flag.yaml | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/testing/kuttl/e2e/backup/08--backup-with-just-trigger.yaml b/testing/kuttl/e2e/backup/08--backup-with-just-trigger.yaml index 9eb4bf1a..687f22b4 100644 --- a/testing/kuttl/e2e/backup/08--backup-with-just-trigger.yaml +++ b/testing/kuttl/e2e/backup/08--backup-with-just-trigger.yaml @@ -1,18 +1,20 @@ apiVersion: kuttl.dev/v1beta1 kind: TestStep commands: -- script: | +- script: | pgbackrest_backup_annotation() { kubectl get --namespace "${NAMESPACE}" postgrescluster/backup-cluster \ --output 'go-template={{ index .metadata.annotations "postgres-operator.crunchydata.com/pgbackrest-backup" }}' } PRIOR=$(pgbackrest_backup_annotation) + # ensure annotation timestamp will be different + sleep 1 RESULT=$(kubectl-pgo --namespace "${NAMESPACE}" backup backup-cluster) CURRENT=$(pgbackrest_backup_annotation) if [ "${CURRENT}" = "${PRIOR}" ]; then - printf 'Expected annotation to change, got %q' "${CURRENT}" + printf 'Expected annotation to change, got %q\n' "${CURRENT}" exit 1 fi diff --git a/testing/kuttl/e2e/backup/11--backup-with-new-flag.yaml b/testing/kuttl/e2e/backup/11--backup-with-new-flag.yaml index 5c418a92..43f61904 100644 --- a/testing/kuttl/e2e/backup/11--backup-with-new-flag.yaml +++ b/testing/kuttl/e2e/backup/11--backup-with-new-flag.yaml @@ -8,11 +8,11 @@ commands: echo "RESULT from taking backup: ${RESULT}" if [ "${STATUS-0}" -eq 0 ]; then - printf 'Expected error, got none' + printf 'Expected error, got none\n' exit 1 fi if [[ "${RESULT,,}" != *conflict* || "${RESULT}" != *repoName* ]]; then - printf 'Expected conflict on repoName, got %q' "${RESULT}" + printf 'Expected conflict on repoName, got %q\n' "${RESULT}" exit 1 fi