Added a new annotation: vertica.com/superuser-name #578
Conversation
| @@ -124,7 +124,7 @@ func (r *VerticaDBReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( | |||
| if err != nil { | |||
| return ctrl.Result{}, err | |||
| } | |||
| prunner := cmds.MakeClusterPodRunner(log, r.Cfg, passwd) | |||
| prunner := cmds.MakeClusterPodRunner(log, r.Cfg, vmeta.GetSuperuserName(vdb.Annotations), passwd) | |||
There was a problem hiding this comment.
Can we use vdb.GetVerticaUser? It will make it easier to switch if we ever convert the annotation to a CR parm.
EDIT: This is a general comment as I see this pattern in quite a few places.
.github/workflows/e2e-leg-4.yml
Outdated
| @@ -72,6 +77,10 @@ jobs: | |||
| export E2E_TEST_DIRS="tests/e2e-leg-4" | |||
| export VERTICA_DEPLOYMENT_METHOD=${{ inputs.vertica-deployment-method }} | |||
| if [ "${VERTICA_DEPLOYMENT_METHOD}" != "vclusterops" ]; then E2E_TEST_DIRS+=" tests/e2e-leg-4-at-only"; fi | |||
| export VERTICA_SUPERUSER_NAME=${{ inputs.vertica-superuser-name }} | |||
| if [ -z "${VERTICA_SUPERUSER_NAME}" -o "${VERTICA_DEPLOYMENT_METHOD}" != "vclusterops" ]; then | |||
| VERTICA_SUPERUSER_NAME="dbadmin" | |||
There was a problem hiding this comment.
Should we export this?
| VERTICA_SUPERUSER_NAME="dbadmin" | |
| export VERTICA_SUPERUSER_NAME="dbadmin" |
| @@ -72,6 +77,10 @@ jobs: | |||
| export E2E_TEST_DIRS="tests/e2e-leg-4" | |||
| export VERTICA_DEPLOYMENT_METHOD=${{ inputs.vertica-deployment-method }} | |||
| if [ "${VERTICA_DEPLOYMENT_METHOD}" != "vclusterops" ]; then E2E_TEST_DIRS+=" tests/e2e-leg-4-at-only"; fi | |||
| export VERTICA_SUPERUSER_NAME=${{ inputs.vertica-superuser-name }} | |||
There was a problem hiding this comment.
I don't think we are setting this correctly. When I look at your CI run I see dbadmin being used:
Vertica server image name: docker.io/vertica/vertica-k8s-private:latest-minimal-master
Base vertica server image name for upgrade tests: <not-set>
Vertica logger image name: vertica-logger:kind
Endpoint: https://minio.kuttl-e2e-s3/
Protocol: s3://
Communal Path Prefix: s3://nimbusdb/
Using private registry: NO
Add server mounts: NO
Deployment method: vclusterops
Vertica superuser name: dbadmin
https://github.com/vertica/vertica-kubernetes/actions/runs/6785933635/job/18448129113
The new parameter was added to workflow_dispatch. That gets called when you trigger just this workflow. There are a set of parameters for workflow_call. This is used when another workflow calls this workflow, which is what happens during a PR. Can you add the new parameter to workflow_call?
| @@ -15,6 +15,9 @@ on: | |||
| vertica-deployment-method: | |||
| type: string | |||
| required: false | |||
| vertica-superuser-name: | |||
| type: string | |||
| required: false | |||
There was a problem hiding this comment.
Should we set the default?
| required: false | |
| required: false | |
| default: "myadmin" |
There was a problem hiding this comment.
I set the default value in workflow_dispatch.
There was a problem hiding this comment.
I don't think it's picking that up though. Workflow dispatch is when you call the workflow directly from the REST API. Workflow call is when it is called by another workflow, which is what we are doing.
This allows a custom superuser name to be used. It builds on the work done in #578. That issue was specifically for vclusterops deployments. This one is specific to admintools deployments. It fixes a few code paths that were specific to admintools deployments. Note: the admintools image hard codes the superuser name as dbadmin, so you will still need a custom image to run with different named superuser. This change is strictly for the operator.
Added a superuser-name annotation. This annotation can be used to customize Vertica superuser name. This should be used with vclusterops. When admintools is used, this annotation will be always set to "dbadmin".