From b1823de1a26e57ba393738c92c6835da75d5602e Mon Sep 17 00:00:00 2001 From: prafull01 Date: Tue, 7 Jan 2025 10:10:02 +0530 Subject: [PATCH] Fix existing tests --- build/templates/values.yaml | 98 +++++++++++++++++++ cockroachdb/values.yaml | 6 +- .../e2e/install/cockroachdb_helm_e2e_test.go | 11 ++- .../rotate/cockroachdb_rotate_certs_test.go | 1 + tests/k3d/dev-cluster.sh | 6 +- .../cockroachdb_helm_template_test.go | 60 ++++++++++-- tests/testutil/require.go | 2 +- 7 files changed, 170 insertions(+), 14 deletions(-) diff --git a/build/templates/values.yaml b/build/templates/values.yaml index 8f02560c..2839ee5a 100644 --- a/build/templates/values.yaml +++ b/build/templates/values.yaml @@ -710,3 +710,101 @@ iap: # Create Google Cloud OAuth credentials and set client id and secret # clientId: # clientSecret: + +# Use the CRDB Operator to manage the CRDB clusters +operator: + enabled: true + # Default values for the cluster chart. + image: + repository: cockroachdb/cockroach + pullPolicy: IfNotPresent + # Overrides the image tag whose default is the cluster chart's appVersion. + tag: "" + + nameOverride: "" + fullnameOverride: "" + + # A map of CRDB cluster settings. + # See https://www.cockroachlabs.com/docs/stable/cluster-settings.html + clusterSettings: ~ + + # Regions controls the number of CRDB nodes that are deployed per region. + # regions: ~ + # - code: us-central1 + # nodes: 3 + + # loggingConf is the logging configuration used by cockroach. + # More details: https://www.cockroachlabs.com/docs/stable/logging-overview.html + loggingConf: ~ + # sinks: + # stderr: + # channels: [health, dev] + # filter: INFO + + # We usually recommend not to specify default resources and to leave this as a conscious + # choice for the user. This also increases chances charts run on environments with little + # resources, such as K3D. If you do want to specify resources, uncomment the following + # lines, adjust them as necessary, and remove the tilde after 'resources:'. + resources: ~ + # limits: + # cpu: 100m + # memory: 128Mi + # requests: + # cpu: 100m + # memory: 128Mi + + certificates: + # Any extra alt names that should be added to the node certs. + extraNodeAltNames: [] + # - somevalue + # - somevalue.default + # - somevalue.default.svc.local + # the number of days generated certs are valid for + # validForDays: 3650 + + # External certificates for the CRDB cluster. + externalCertificates: + clientCaConfigMapName: my-release-cockroachdb-ca-secret-crt + nodeCaConfigMapName: my-release-cockroachdb-ca-secret-crt + httpSecretName: my-release-cockroachdb-client-secret + nodeClientSecretName: my-release-cockroachdb-client-secret + nodeSecretName: my-release-cockroachdb-node-secret + rootSqlClientSecretName: my-release-cockroachdb-client-secret + + # RBAC settings for CRDB nodes + rbac: + # By default the service account will be the resource name. It will + # be created during the installation along with a namespaced role and + # a cluster role with the policy rules below. + # + # Uncomment the line below to use a custom SA. If a custom SA is used, + # no roles or bindings will be created. + # serviceAccountName: my-custom-sa + + # Rules for the namespaced role bound to the service account. + # + # E.g. + # permissions: + # - apiGroup: [""] + # resources: ["secrets"] + # verbs: ["create", "get"] + rules: [] + + # Rules for the cluster role bound to the service account. + clusterRules: + # Get nodes allows the locality container to work as expected. It pulls the + # failure-domain.beta.kubernetes.io/zone label to determine node locality. + - apiGroups: [""] + resources: ["nodes"] + verbs: ["get"] + serviceAccountName: ~ + + regions: + - code: us-east-1 + nodes: 3 + cloudProvider: k3d + namespace: default + + extras: + # Add a container with dnsutils (nslookup, dig, ping, etc.) installed. + dnsutils: false diff --git a/cockroachdb/values.yaml b/cockroachdb/values.yaml index 8efd788d..db12e209 100644 --- a/cockroachdb/values.yaml +++ b/cockroachdb/values.yaml @@ -730,7 +730,7 @@ operator: clusterSettings: ~ # Regions controls the number of CRDB nodes that are deployed per region. - #regions: ~ + # regions: ~ # - code: us-central1 # nodes: 3 @@ -764,7 +764,7 @@ operator: # validForDays: 3650 # External certificates for the CRDB cluster. - externalCertificates: + externalCertificates: clientCaConfigMapName: my-release-cockroachdb-ca-secret-crt nodeCaConfigMapName: my-release-cockroachdb-ca-secret-crt httpSecretName: my-release-cockroachdb-client-secret @@ -799,7 +799,7 @@ operator: resources: ["nodes"] verbs: ["get"] serviceAccountName: ~ - + regions: - code: us-east-1 nodes: 3 diff --git a/tests/e2e/install/cockroachdb_helm_e2e_test.go b/tests/e2e/install/cockroachdb_helm_e2e_test.go index 46c4bc81..5edf994a 100644 --- a/tests/e2e/install/cockroachdb_helm_e2e_test.go +++ b/tests/e2e/install/cockroachdb_helm_e2e_test.go @@ -60,6 +60,7 @@ func TestCockroachDbHelmInstall(t *testing.T) { options := &helm.Options{ KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), SetValues: patchHelmValues(map[string]string{ + "operator.enabled": "false", "conf.cluster-name": "test", "init.provisioning.enabled": "true", "init.provisioning.databases[0].name": testDBName, @@ -146,6 +147,7 @@ func TestCockroachDbHelmInstallWithCAProvided(t *testing.T) { options := &helm.Options{ KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), SetValues: patchHelmValues(map[string]string{ + "operator.enabled": "false", "tls.certs.selfSigner.caProvided": "true", "tls.certs.selfSigner.caSecret": customCASecret, }), @@ -273,6 +275,7 @@ func TestCockroachDbHelmMigration(t *testing.T) { options := &helm.Options{ KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), SetValues: patchHelmValues(map[string]string{ + "operator.enabled": "false", "tls.certs.provided": "true", "tls.certs.selfSigner.enabled": "false", "tls.certs.clientRootSecret": crdbCluster.ClientSecret, @@ -306,6 +309,7 @@ func TestCockroachDbHelmMigration(t *testing.T) { options = &helm.Options{ KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), SetValues: patchHelmValues(map[string]string{ + "operator.enabled": "false", "statefulset.updateStrategy.type": "OnDelete", }), ExtraArgs: map[string][]string{ @@ -363,7 +367,8 @@ func TestCockroachDbWithInsecureMode(t *testing.T) { options := &helm.Options{ KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), SetValues: patchHelmValues(map[string]string{ - "tls.enabled": "false", + "operator.enabled": "false", + "tls.enabled": "false", }), } @@ -453,6 +458,7 @@ spec: options := &helm.Options{ KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), SetValues: patchHelmValues(map[string]string{ + "operator.enabled": "false", "tls.enabled": "true", "tls.certs.selfSigner.enabled": "false", "tls.certs.certManager": "true", @@ -485,6 +491,7 @@ func TestWALFailoverSideDiskExistingCluster(t *testing.T) { testWALFailoverExistingCluster( t, patchHelmValues(map[string]string{ + "operator.enabled": "false", "conf.wal-failover.value": "path=cockroach-failover", "conf.wal-failover.persistentVolume.enabled": "true", "conf.wal-failover.persistentVolume.size": "1Gi", @@ -496,6 +503,7 @@ func TestWALFailoverAmongStoresExistingCluster(t *testing.T) { testWALFailoverExistingCluster( t, patchHelmValues(map[string]string{ + "operator.enabled": "false", "conf.wal-failover.value": "among-stores", "conf.store.count": "2", }), @@ -530,6 +538,7 @@ func testWALFailoverExistingCluster(t *testing.T, additionalValues map[string]st // Configure options for the initial deployment. initialValues := patchHelmValues(map[string]string{ + "operator.enabled": "false", "conf.cluster-name": "test", "conf.store.enabled": "true", "statefulset.replicas": strconv.Itoa(numReplicas), diff --git a/tests/e2e/rotate/cockroachdb_rotate_certs_test.go b/tests/e2e/rotate/cockroachdb_rotate_certs_test.go index dca1478b..56f4b2d6 100644 --- a/tests/e2e/rotate/cockroachdb_rotate_certs_test.go +++ b/tests/e2e/rotate/cockroachdb_rotate_certs_test.go @@ -59,6 +59,7 @@ func TestCockroachDbRotateCertificates(t *testing.T) { // Setup the args. For this test, we will set the following input values: helmValues := map[string]string{ + "operator.enabled": "false", "tls.selfSigner.image.tag": tagOutput, "storage.persistentVolume.size": "1Gi", "tls.certs.selfSigner.minimumCertDuration": "24h", diff --git a/tests/k3d/dev-cluster.sh b/tests/k3d/dev-cluster.sh index 830d2595..6da05493 100755 --- a/tests/k3d/dev-cluster.sh +++ b/tests/k3d/dev-cluster.sh @@ -2,6 +2,8 @@ region="us-east-1" zones=3 +K3D_PATH="../../bin/k3d" + if [ $# -eq 0 ] then echo "No arguments supplied: " @@ -58,7 +60,7 @@ set_node_labels() { case $COMMAND in up) node_labels=$(set_node_labels ${nodes} ${region} ${zones}) - k3d cluster create ${name} \ + $(K3D_PATH) cluster create ${name} \ --network ${network_name} \ --registry-config "$SCRIPT_DIR/registries.yaml" \ --image rancher/k3s:v${version}-k3s1 \ @@ -67,7 +69,7 @@ case $COMMAND in ${node_labels} ;; down) - k3d cluster delete ${name} + $(K3D_PATH) cluster delete ${name} ;; *) echo "Unknown command: $COMMAND" diff --git a/tests/template/cockroachdb_helm_template_test.go b/tests/template/cockroachdb_helm_template_test.go index a7f43ce1..9afc902f 100644 --- a/tests/template/cockroachdb_helm_template_test.go +++ b/tests/template/cockroachdb_helm_template_test.go @@ -36,7 +36,6 @@ func init() { // TestTLSEnable tests the enabling the TLS, you have to enable only one method of TLS certs func TestTLSEnable(t *testing.T) { t.Parallel() - testCases := []struct { name string values map[string]string @@ -47,6 +46,7 @@ func TestTLSEnable(t *testing.T) { map[string]string{ "tls.certs.selfSigner.enabled": "false", "tls.certs.certManager": "false", + "operator.enabled": "false", }, "You have to enable either self signed certificates or certificate manager, if you have enabled tls", }, @@ -55,6 +55,7 @@ func TestTLSEnable(t *testing.T) { map[string]string{ "tls.certs.selfSigner.enabled": "true", "tls.certs.certManager": "true", + "operator.enabled": "false", }, "Can not enable the self signed certificates and certificate manager at the same time", }, @@ -356,7 +357,10 @@ func TestHelmSelfCertSignerStatefulSet(t *testing.T) { }{ { "Self Signer enable", - map[string]string{"tls.certs.selfSigner.enabled": "true"}, + map[string]string{ + "tls.certs.selfSigner.enabled": "true", + "operator.enabled": "false", + }, "copy-certs", }, { @@ -364,6 +368,7 @@ func TestHelmSelfCertSignerStatefulSet(t *testing.T) { map[string]string{ "tls.certs.selfSigner.enabled": "false", "tls.certs.certManager": "true", + "operator.enabled": "false", }, "copy-certs", }, @@ -511,7 +516,10 @@ func TestHelmLogConfigFileStatefulSet(t *testing.T) { }{ { "New logging configuration enabled", - map[string]string{"conf.log.enabled": "true"}, + map[string]string{ + "conf.log.enabled": "true", + "operator.enabled": "false", + }, expect{ "--log-config-file=/cockroach/log-config/log-config.yaml", "", @@ -525,6 +533,7 @@ func TestHelmLogConfigFileStatefulSet(t *testing.T) { map[string]string{ "conf.log.enabled": "true", "conf.log.config.file-defaults.dir": "/cockroach/cockroach-logs", + "operator.enabled": "false", }, expect{ "--log-config-file=/cockroach/log-config/log-config.yaml", @@ -539,6 +548,7 @@ func TestHelmLogConfigFileStatefulSet(t *testing.T) { map[string]string{ "conf.log.enabled": "false", "conf.logtostderr": "INFO", + "operator.enabled": "false", }, expect{ "--logtostderr=INFO", @@ -554,6 +564,7 @@ func TestHelmLogConfigFileStatefulSet(t *testing.T) { "conf.log.enabled": "false", "conf.logtostderr": "INFO", "conf.log.persistentVolume.enabled": "true", + "operator.enabled": "false", }, expect{ "--logtostderr=INFO", @@ -569,6 +580,7 @@ func TestHelmLogConfigFileStatefulSet(t *testing.T) { "conf.log.enabled": "true", "conf.log.config.file-defaults.dir": "/wrong/path", "conf.log.persistentVolume.enabled": "true", + "operator.enabled": "false", }, expect{ "", @@ -584,6 +596,7 @@ func TestHelmLogConfigFileStatefulSet(t *testing.T) { "conf.log.enabled": "true", "conf.log.config.file-defaults.dir": "/cockroach/cockroach-logs", "conf.log.persistentVolume.enabled": "true", + "operator.enabled": "false", }, expect{ "--log-config-file=/cockroach/log-config/log-config.yaml", @@ -671,7 +684,10 @@ func TestHelmDatabaseProvisioning(t *testing.T) { }{ { "Disabled provisioning", - map[string]string{"init.provisioning.enabled": "false"}, + map[string]string{ + "init.provisioning.enabled": "false", + "operator.enabled": "false", + }, struct { job struct { exists bool @@ -712,7 +728,10 @@ func TestHelmDatabaseProvisioning(t *testing.T) { }, { "Enabled empty provisioning", - map[string]string{"init.provisioning.enabled": "true"}, + map[string]string{ + "init.provisioning.enabled": "true", + "operator.enabled": "false", + }, struct { job struct { exists bool @@ -758,6 +777,7 @@ func TestHelmDatabaseProvisioning(t *testing.T) { "init.provisioning.users[0].name": "testUser", "init.provisioning.users[0].password": "testPassword", "init.provisioning.users[0].options[0]": "CREATEROLE", + "operator.enabled": "false", }, struct { job struct { @@ -804,6 +824,7 @@ func TestHelmDatabaseProvisioning(t *testing.T) { "init.provisioning.enabled": "true", "init.provisioning.databases[0].name": "testDatabase", "init.provisioning.databases[0].options[0]": "encoding='utf-8'", + "operator.enabled": "false", }, struct { job struct { @@ -851,6 +872,7 @@ func TestHelmDatabaseProvisioning(t *testing.T) { "init.provisioning.users[0].password": "testPassword", "init.provisioning.databases[0].name": "testDatabase", "init.provisioning.databases[0].owners[0]": "testUser", + "operator.enabled": "false", }, struct { job struct { @@ -900,6 +922,7 @@ func TestHelmDatabaseProvisioning(t *testing.T) { "init.provisioning.enabled": "true", "init.provisioning.clusterSettings.cluster\\.organization": "testOrganization", "init.provisioning.clusterSettings.enterprise\\.license": "testLicense", + "operator.enabled": "false", }, struct { job struct { @@ -953,6 +976,7 @@ func TestHelmDatabaseProvisioning(t *testing.T) { "init.provisioning.databases[0].backup.recurring": "@always", "init.provisioning.databases[0].backup.fullBackup": "@daily", "init.provisioning.databases[0].backup.schedule.options[0]": "first_run = 'now'", + "operator.enabled": "false", }, struct { job struct { @@ -1297,7 +1321,9 @@ func TestHelmInitJobAnnotations(t *testing.T) { }{ { "No extra job annotations were supplied", - map[string]string{}, + map[string]string{ + "operator.enabled": "false", + }, map[string]string{ "helm.sh/hook": "post-install,post-upgrade", "helm.sh/hook-delete-policy": "before-hook-creation", @@ -1308,6 +1334,7 @@ func TestHelmInitJobAnnotations(t *testing.T) { map[string]string{ "init.jobAnnotations.test-key-1": "test-value-1", "init.jobAnnotations.test-key-2": "test-value-2", + "operator.enabled": "false", }, map[string]string{ "helm.sh/hook": "post-install,post-upgrade", @@ -1364,6 +1391,7 @@ func TestStatefulSetInitContainers(t *testing.T) { "statefulset.volumeMounts[0].mountPath": "/metadata", "statefulset.volumes[0].name": "metadata", "statefulset.volumes[0].configMap.name": "log-config", + "operator.enabled": "false", }, }, { @@ -1375,6 +1403,7 @@ func TestStatefulSetInitContainers(t *testing.T) { "statefulset.volumeMounts[0].mountPath": "/metadata", "statefulset.volumes[0].name": "metadata", "statefulset.volumes[0].configMap.name": "log-config", + "operator.enabled": "false", }, }, { @@ -1387,6 +1416,7 @@ func TestStatefulSetInitContainers(t *testing.T) { "statefulset.initContainers[0].command[0]": "/bin/bash", "statefulset.initContainers[0].command[1]": "-c", "statefulset.initContainers[0].command[2]": "echo 'Fetching metadata'", + "operator.enabled": "false", }, }, } @@ -1465,6 +1495,7 @@ func TestHelmCockroachStartCmd(t *testing.T) { "start single node with default args", map[string]string{ "conf.single-node": "true", + "operator.enabled": "false", }, expect{ "exec /cockroach/cockroach start-single-node " + @@ -1492,6 +1523,7 @@ func TestHelmCockroachStartCmd(t *testing.T) { "conf.locality": "region=us-west-1", "conf.sql-audit-dir": "/audit", "conf.log.enabled": "true", + "operator.enabled": "false", }, expect{ "exec /cockroach/cockroach start-single-node " + @@ -1512,7 +1544,8 @@ func TestHelmCockroachStartCmd(t *testing.T) { { "start multiple node cluster with default args", map[string]string{ - "conf.join": "1.1.1.1", + "conf.join": "1.1.1.1", + "operator.enabled": "false", }, expect{ "exec /cockroach/cockroach start --join=1.1.1.1 " + @@ -1542,6 +1575,7 @@ func TestHelmCockroachStartCmd(t *testing.T) { "conf.locality": "region=us-west-1", "conf.sql-audit-dir": "/audit", "conf.log.enabled": "true", + "operator.enabled": "false", }, expect{ "exec /cockroach/cockroach start --join=1.1.1.1 " + @@ -1602,6 +1636,7 @@ func TestHelmCockroachStartCmd(t *testing.T) { // TestHelmWALFailoverConfiguration contains the tests around WAL failover configuration. func TestHelmWALFailoverConfiguration(t *testing.T) { t.Parallel() + t.Logf("helm chart path: %s", helmChartPath) type expect struct { statefulsetArgument string @@ -1618,6 +1653,7 @@ func TestHelmWALFailoverConfiguration(t *testing.T) { "WAL failover invalid configuration", map[string]string{ "conf.wal-failover.value": "invalid", + "operator.enabled": "false", }, expect{ "", @@ -1631,6 +1667,7 @@ func TestHelmWALFailoverConfiguration(t *testing.T) { "conf.wal-failover.value": "", "conf.store.enabled": "true", "conf.store.count": "1", + "operator.enabled": "false", }, expect{ "--store=path=cockroach-data,size=100Gi", @@ -1644,6 +1681,7 @@ func TestHelmWALFailoverConfiguration(t *testing.T) { "conf.wal-failover.value": "among-stores", "conf.store.enabled": "true", "conf.store.count": "2", + "operator.enabled": "false", }, expect{ "--store=path=cockroach-data,size=100Gi " + @@ -1659,6 +1697,7 @@ func TestHelmWALFailoverConfiguration(t *testing.T) { "conf.wal-failover.value": "disabled", "conf.store.enabled": "true", "conf.store.count": "2", + "operator.enabled": "false", }, expect{ "--store=path=cockroach-data,size=100Gi " + @@ -1673,6 +1712,7 @@ func TestHelmWALFailoverConfiguration(t *testing.T) { map[string]string{ "conf.wal-failover.value": "among-stores", "conf.store.enabled": "false", + "operator.enabled": "false", }, expect{ "", @@ -1686,6 +1726,7 @@ func TestHelmWALFailoverConfiguration(t *testing.T) { "conf.wal-failover.value": "among-stores", "conf.store.enabled": "true", "conf.store.count": "1", + "operator.enabled": "false", }, expect{ "", @@ -1698,6 +1739,7 @@ func TestHelmWALFailoverConfiguration(t *testing.T) { map[string]string{ "conf.wal-failover.value": "path=/cockroach/cockroach-failover/abc", "conf.wal-failover.persistentVolume.enabled": "true", + "operator.enabled": "false", }, expect{ "--wal-failover=path=/cockroach/cockroach-failover/abc", @@ -1710,6 +1752,7 @@ func TestHelmWALFailoverConfiguration(t *testing.T) { map[string]string{ "conf.wal-failover.value": "path=cockroach-failover/abc", "conf.wal-failover.persistentVolume.enabled": "true", + "operator.enabled": "false", }, expect{ "--wal-failover=path=cockroach-failover/abc", @@ -1722,6 +1765,7 @@ func TestHelmWALFailoverConfiguration(t *testing.T) { map[string]string{ "conf.wal-failover.value": "disabled", "conf.wal-failover.persistentVolume.enabled": "true", + "operator.enabled": "false", }, expect{ "--wal-failover=disabled", @@ -1734,6 +1778,7 @@ func TestHelmWALFailoverConfiguration(t *testing.T) { map[string]string{ "conf.wal-failover.value": "path=/cockroach/cockroach-failover", "conf.wal-failover.persistentVolume.enabled": "false", + "operator.enabled": "false", }, expect{ "", @@ -1746,6 +1791,7 @@ func TestHelmWALFailoverConfiguration(t *testing.T) { map[string]string{ "conf.wal-failover.value": "path=/invalid", "conf.wal-failover.persistentVolume.enabled": "true", + "operator.enabled": "false", }, expect{ "", diff --git a/tests/testutil/require.go b/tests/testutil/require.go index dc847941..46bbe0b1 100644 --- a/tests/testutil/require.go +++ b/tests/testutil/require.go @@ -455,7 +455,7 @@ func WaitUntilPodDeleted( if err != nil && kube.IsNotFound(err) { return "Pod is now deleted", nil } - return "", errors.New(fmt.Sprintf("pod is not deleted: %s", err)) + return "", fmt.Errorf("pod is not deleted: %s", err) }, ) if err != nil {