Page MenuHomeElementl

[1/n] Add helm schema validation for Dagit configuration
ClosedPublic

Authored by rexledesma on Oct 24 2020, 12:58 AM.

Details

Summary

This sets up the boilerplate for validating the values.yaml file using a values.schema.json file. The schema file is created through pydantic's DSL. I've also added a cli tool to update the helm repo's schema. The workflow for updating the schema should look something like this:

  1. Make changes to pydantic models
  2. Show changes using dagster-helm schema | git diff --no-index --exit-code -- helm/dagster/values.schema.json -
  3. If the changes are good, dagster-helm schema --command=apply to update the current values.schema.json.

As an example, we add schema validation for the helm values that configure the Dagit webserver.

Test Plan

helm lint helm/dagster -f helm/dagster/values.yaml, bk

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

helm/dagster/values.schema.json
88

Do we advertise a specific Kubernetes that we support? I used 1.15.0 since our python version of 11.0.0 k8s has feature parity with it.

88

Also, these definitions are a god send. Was in the middle of implementing some of them by hand when I realized that there's probably some community contributed source for these.

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 24 2020, 1:19 AM
Harbormaster failed remote builds in B20118: Diff 24415!
helm/dagster/values.schema.json
41

These properties (env, env_config_maps, env_secrets) should really follow the schema of the configs they model in kubernetes (e.g. env_config_maps -> https://kubernetesjsonschema.dev/v1.15.0/_definitions.json#/definitions/io.k8s.api.core.v1.EnvFromSource). Then, there will be no need to wrangle these values into their required form - they can simply be interpolated with a simple nindent.

TEST PLAN
helm lint helm/dagster -f helm/dagster/values.yaml

does helm only use these values to power lint or is it a deeper integration?

helm/dagster/values.schema.json
2

I feel like this should likely be the output of a program instead of something we manually manage - doing that will make it much easier to expand to all the other parts of the schema

41

will these changes be breaking changes? if so queue them up for 0.10.0

88

im not sure which version we should pin to - but moving this to be the output of a program should make it much easier to manage / change that out

helm/dagster/values.yaml
314

🥴

rexledesma added inline comments.
helm/dagster/values.schema.json
2

Good idea - here's a nice DSL that we can use: https://pydantic-docs.helpmanual.io/usage/schema/ - I'll put some generator under python_modules/automation/automation/helm

  • Adds dagster-helm cli to programatically generate schema file
  • Add buildkite from D4897
rexledesma retitled this revision from Add helm schema validation for Dagit configuration to [1/n] Add helm schema validation for Dagit configuration.Oct 27 2020, 10:57 PM
rexledesma edited the summary of this revision. (Show Details)
rexledesma edited the test plan for this revision. (Show Details)

@alangenfeld Thanks for the suggestion - I added a cli tool to generate the schema file. Also, the buildkite step that was added will ensure the following things:

  1. dagster-helm schema --command=apply was checked into git if there was change to the pydantic schema
  2. Our values.yaml is well formed according to our generated schema. This makes use of a deeper integration in helm, so that we can apply the schema.
rexledesma added inline comments.
helm/dagster/values.schema.json
41

I'll refactor these properties as camelcase + use the new $ref schema in a separate diff.

redfordnod

.buildkite/pipeline.py
709

| here feels odd to me - would this be clearer if it was just two separate commands?

a blanket git diff --exit-code also is a bit more future proof incase we start generating other outputs

python_modules/automation/automation/helm/schema/dagit.py
20–31

ya this is really nice

python_modules/automation/automation/helm/schema/kubernetes.py
33

ah inner class shenanigans

This revision is now accepted and ready to land.Oct 28 2020, 3:45 PM