Deploying VTAdmin in Vtop by GuptaManan100 · Pull Request #272 · planetscale/vitess-operator

GuptaManan100 · 2022-06-21T16:55:12Z

Description

This PR adds VTAdmin to the operator and associated tests.
Currently only the specific vtadmin image can be used for deploying VTAdmin, but we'll get vitess/lite image working too later.

An example config looks like the following -

spec:
  images:
    vtadmin: vitess/vtadmin:latest
  vtadmin:
    rbac:
      name: example-cluster-config
      key: rbac.yaml
    cells:
      - zone1
    apiAddresses:
      - http://localhost:14001
    replicas: 1
    readOnly: false
    apiResources:
      limits:
        memory: 128Mi
      requests:
        cpu: 100m
        memory: 128Mi
    webResources:
      limits:
        memory: 128Mi
      requests:
        cpu: 100m
        memory: 128Mi

The VTAdmin spec includes the following fields -

Rbac as a secret source. It is the role based access control rules to use for VTAdmin API
Cells as a list of strings. It is the cells where VTAdmin must be deployed. Defaults to deploying everywhere
APIAddresses as a list of strings. Since the VTAdmin web UI runs on the client side, it needs the API address to use to query the API. We can't use the internal kubernetes service address or the ip of the pod, since they aren't visible outside the cluster. The API Address must be provided by the user based on how they export the service of VTAdmin API. In our tests we port-forward the service to port 14001 and therefore that is the address provided here. This value is a list because the address to be used for VTAdmin web in each cell might be different. If only 1 value is provided then, that is used for all the cells. The ideal way to deploy this would be to export each individual VTAdmin service (that we create) in each cell and attach external IPs to them and provide those IPs here.
Replicas - Number of VTAdmin deployments required per cell. We setup a service on top of the web and API connections, so load-balancing comes out of the box
WebResources - Resource requests and limits for the container running the VTAdmin web server
APIResources - Resource requests and limits for the container running the VTAdmin API server
ReadOnly - Configuration to set the VTAdmin web UI to be read only.

Documentation

vitessio/website#1073

…y crds Signed-off-by: Manan Gupta <manan@planetscale.com>

Signed-off-by: Manan Gupta <manan@planetscale.com>

…create a keyspace Signed-off-by: Manan Gupta <manan@planetscale.com>

Signed-off-by: Manan Gupta <manan@planetscale.com>

…nds in the docker image of vtadmin Signed-off-by: Manan Gupta <manan@planetscale.com>

docs/api/index.html

…nt variables Signed-off-by: Manan Gupta <manan@planetscale.com>

Signed-off-by: Manan Gupta <manan@planetscale.com>

GuptaManan100 · 2022-06-27T14:33:14Z

.buildkite/pipeline.yml

+
+  - name: "VTAdmin Test"
+    command:
+    - apk add g++ make bash curl mysql mysql-client chromium


This line is different from the other test setup lines. It is also installing chromium browser. We need this to run it in a headless mode to download the VTAdmin web pages. We can't use curl (which we use for the API pages) or wget because they do not support JavaScript, so a headless browser was the easiest solution.

GuptaManan100 · 2022-06-27T14:35:37Z

pkg/controller/vitesscluster/reconcile_vtadmin.go

+	if len(vt.Spec.Images.Vtadmin) == 0 {
+		log.Error("Not deploying vtadmin since image is unspecified")
+		return resultBuilder.Result()
+	}
+
+	if len(vt.Spec.VtAdmin.APIAddresses) == 0 {
+		log.Errorf("Not deploying vtadmin since api addresses field is not specified. Atleast 1 value is required")
+	}
+
+	if len(vt.Spec.VtAdmin.APIAddresses) != 1 && len(vt.Spec.VtAdmin.APIAddresses) != len(vt.Spec.VtAdmin.Cells) {
+		log.Errorf("Not deploying vtadmin since api addresses field doesn't align with cells field")
+	}


These are user input validations. There exist 3 -

The VTAdmin image should be specifically provided. We do not add a default for vitess:lite since that won't work

There should be at least 1 APIAddress provided

If more than 1 APIAddresses are provided, then the length of this list should match the length of the cells list.

That makes sense to me

GuptaManan100 · 2022-06-27T14:36:07Z

pkg/controller/vitesscluster/reconcile_vtadmin.go

+		// We have already checked that atleast 1 value should be available in APIAddresses
+		apiAddress := vt.Spec.VtAdmin.APIAddresses[0]
+		if len(vt.Spec.VtAdmin.APIAddresses) > 1 {
+			apiAddress = vt.Spec.VtAdmin.APIAddresses[idx]
+		}


This is safe because of the user input validation above

GuptaManan100 · 2022-06-27T14:37:37Z

pkg/controller/vitesscluster/reconcile_vtadmin.go

+	discoveryVal := fmt.Sprintf(`{
+    "vtctlds": [
+        {
+            "host": {
+                "fqdn": "%s:%d",
+                "hostname": "%s:%d"
+            }
+        }
+    ],
+    "vtgates": [
+        {
+            "host": {
+                "hostname": "%s:%d"
+            }
+        }
+    ]
+}`, vtctldServiceIP, vtctldServiceWebPort, vtctldServiceIP, vtctldServiceGrpcPort, vtgateServiceIP, vtgateServiceGrpcPort)
+	secretName := vtadmin.DiscoverySecretName(vt.Name, cell.Name)


The discovery configuration file is autogenerated using the global service of vtctld and per cell service of vtgates.

GuptaManan100 · 2022-06-27T14:41:56Z

pkg/controller/vitesscluster/reconcile_vtadmin.go

+	configKey := vtadmin.WebConfigFileName
+	configVal := fmt.Sprintf(`window.env = {
+     'REACT_APP_VTADMIN_API_ADDRESS': "%s",
+     'REACT_APP_FETCH_CREDENTIALS': "omit",
+     'REACT_APP_ENABLE_EXPERIMENTAL_TABLET_DEBUG_VARS': false,
+     'REACT_APP_BUGSNAG_API_KEY': "",
+     'REACT_APP_DOCUMENT_TITLE': "",
+     'REACT_APP_READONLY_MODE': %s,
+ };`, apiAddress, convertReadOnlyFieldToString(vt.Spec.VtAdmin.ReadOnly))
+	secretName := vtadmin.WebConfigSecretName(vt.Name, cell.Name)


Some parameters of the config are not user controllable like REACT_APP_ENABLE_EXPERIMENTAL_TABLET_DEBUG_VARS is always set to false. Since we do not have a good way of discovering vttablets yet, this doesn't work. http-tablet-url-tmpl is not flexible to allow this right now. To enable this, we would have to setup services for all the vttablets and then encode them in the flag. Other complexities like new vttablets requiring change in config, etc will also have to be dealt with.
Similarly, REACT_APP_FETCH_CREDENTIALS is always set to omit because we aren't enabling CORS. If we did want to do that, then we would also need to ask the user to provide the hosts that are allowed to access the API server

GuptaManan100 · 2022-06-27T14:43:04Z

pkg/operator/secrets/volume_mount.go

+	// AbsolutePath stores the absolute path to use instead of the generated path
+	// with /vt/secret as the prefix
+	AbsolutePath string


This had to be added to allow mounting files to a specific path instead of the /vt/secrets where they usually are. For the Vtadmin web config, the path has to be /vt/web/vtadmin/build/config

GuptaManan100 · 2022-06-27T14:43:36Z

pkg/operator/vtadmin/deployment.go

+	updateRbac(spec, apiFlags, vtadminAPIContainer, &obj.Spec.Template.Spec)
+	updateDiscovery(spec, apiFlags, vtadminAPIContainer, &obj.Spec.Template.Spec)


Adds the file mounts for Rbac and discovery config

GuptaManan100 · 2022-06-27T14:43:53Z

pkg/operator/vtadmin/deployment.go

+		VolumeMounts: spec.ExtraVolumeMounts,
+		Env:          spec.ExtraEnv,
+	}
+	updateWebConfig(spec, vtadminWebContainer, &obj.Spec.Template.Spec)


Adds the file mount for the web config

GuptaManan100 · 2022-06-27T14:46:04Z

Also, the vtadmin test needs to restarted after the image vitess/vtadmin:latest is pushed to docker hub. The associated PR that introduces this docker image is vitessio/vitess#10543

deploy/crds/planetscale.com_vitessclusters.yaml

…with root Signed-off-by: Manan Gupta <manan@planetscale.com>

Signed-off-by: Manan Gupta <manan@planetscale.com>

docs/api/index.html

frouioui · 2022-06-27T17:25:41Z

pkg/controller/vitesscluster/reconcile_vtadmin.go

+	if len(vt.Spec.Images.Vtadmin) == 0 {
+		log.Error("Not deploying vtadmin since image is unspecified")
+		return resultBuilder.Result()
+	}
+
+	if len(vt.Spec.VtAdmin.APIAddresses) == 0 {
+		log.Errorf("Not deploying vtadmin since api addresses field is not specified. Atleast 1 value is required")
+	}
+
+	if len(vt.Spec.VtAdmin.APIAddresses) != 1 && len(vt.Spec.VtAdmin.APIAddresses) != len(vt.Spec.VtAdmin.Cells) {
+		log.Errorf("Not deploying vtadmin since api addresses field doesn't align with cells field")
+	}


That makes sense to me

pkg/controller/vitesscluster/reconcile_vtadmin.go

test/endtoend/operator/101_initial_cluster_vtadmin.yaml

frouioui · 2022-06-27T17:31:42Z

pkg/controller/vitesscluster/reconcile_vtadmin.go

+		log.Errorf("Not deploying vtadmin since api addresses field doesn't align with cells field")
+	}
+
+	key := client.ObjectKey{Namespace: vt.Namespace, Name: vtadmin.ServiceName(vt.Name)}


can vtadmin have no service name? if it's not deployed under a k8s service for example

That is a function which deterministically gives the service name to use. We are creating the service just down below. We always create services for vtctld and vtgates in cells, so that if there are multiple deployments then users can just assign a public IP address to the service and it will load balance across all the available pods

frouioui · 2022-06-27T17:37:06Z

pkg/operator/vtadmin/deployment.go

+			// Symlinks are required because the web config file is mounted as a
+			// secret which happens to be mounted as symlink instead of an actual file
+			"--symlinks",


GuptaManan100 added 13 commits June 21, 2022 14:15

feat: add vtadmin specification that the user will provided and modif…

e1bb0fe

…y crds Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: add vtadmin internal specification and reconciliation logic

2a1c016

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: add default for vtadmin image

5e1a6bf

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: add vtadmin test and fix bugs

51be7e6

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: augment vtadmin test by adding more api calls

191b130

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: vtctld should also register vtctld service

024488d

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: bug fix - use the correct IP address

3d380b8

Signed-off-by: Manan Gupta <manan@planetscale.com>

test: augment vtadmin test to query 2 more apis

899d03a

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: fix vtadmin spec json configuration name

471b189

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: fixup cluster name setting of vtadmin and augment test to also …

a3ddf12

…create a keyspace Signed-off-by: Manan Gupta <manan@planetscale.com>

test: add rbac rules and verify that they are honored

254725e

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: add preliminary code to deploy web of vtadmin

90e7fef

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: set the correct environment variables and use the default comma…

b59ed44

…nds in the docker image of vtadmin Signed-off-by: Manan Gupta <manan@planetscale.com>

deepthi reviewed Jun 26, 2022

View reviewed changes

docs/api/index.html Outdated Show resolved Hide resolved

docs/api/index.html Outdated Show resolved Hide resolved

docs/api/index.html Outdated Show resolved Hide resolved

docs/api/index.html Outdated Show resolved Hide resolved

GuptaManan100 added 6 commits June 27, 2022 14:41

feat: move to specifying config file as a volume instead of environme…

fb65f2f

…nt variables Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: add more parameters to vtadmin spec and fix documentation

b43e879

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: make changes to operator to use the newly defined parameters

dcfdd3a

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: use correct endpoints for readiness and liveness check

d0169d5

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: code cleanup and comment

cd8f1d4

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: use chromium to test that vtadmin web is setup correctly

589351a

Signed-off-by: Manan Gupta <manan@planetscale.com>

GuptaManan100 commented Jun 27, 2022

View reviewed changes

GuptaManan100 requested review from Verolop, deepthi, frouioui and notfelineit June 27, 2022 14:46

GuptaManan100 marked this pull request as ready for review June 27, 2022 14:46

GuptaManan100 changed the title ~~Vtadmin in vtop~~ Deploying VTAdmin in Vtop Jun 27, 2022

L3o-pold reviewed Jun 27, 2022

View reviewed changes

deploy/crds/planetscale.com_vitessclusters.yaml Outdated Show resolved Hide resolved

test: add no-sandbox flag as it is required to run headless chromium …

1386d66

…with root Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: rename vtAdmin option to vtadmin

578e28e

Signed-off-by: Manan Gupta <manan@planetscale.com>

frouioui mentioned this pull request Jun 27, 2022

Release of v14.0.0 vitessio/vitess#10476

Closed

43 tasks

frouioui reviewed Jun 27, 2022

View reviewed changes

frouioui self-requested a review June 28, 2022 06:06

GuptaManan100 mentioned this pull request Jun 28, 2022

Add documentation on how to run VTAdmin in VTop vitessio/website#1073

Merged

frouioui approved these changes Jun 28, 2022

View reviewed changes

GuptaManan100 merged commit 6c27c87 into main Jun 28, 2022

GuptaManan100 deleted the vtadmin-in-vtop branch June 28, 2022 10:48

		updateRbac(spec, apiFlags, vtadminAPIContainer, &obj.Spec.Template.Spec)
		updateDiscovery(spec, apiFlags, vtadminAPIContainer, &obj.Spec.Template.Spec)

Conversation

GuptaManan100 commented Jun 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Documentation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuptaManan100 commented Jun 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

GuptaManan100 commented Jun 21, 2022 •

edited

Loading

GuptaManan100 commented Jun 27, 2022 •

edited

Loading