Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/pod value mount #11145

Merged
merged 7 commits into from
Feb 5, 2020
Merged

Conversation

ycliuhw
Copy link
Member

@ycliuhw ycliuhw commented Jan 23, 2020

Please provide the following details to expedite Pull Request review:

Checklist

  • Checked if it requires a pylibjuju change?
  • Added integration tests for the PR?
  • Added or updated doc.go related to packages changed?
  • Do comments answer the question of why design decisions were made?

Description of change

Introduce config options in k8sspec for environment value mount using Env and EnvFrom.

QA steps

deploy a CaaS application with below k8s spec format;

containers:
  - name: gitlab
    image: gitlab/latest
    config:
      attr: foo=bar; name["fred"]="blogs";
      foo: bar
      brackets: '["hello", "world"]'
      restricted: 'yes'
      switch: on
      bar: true
      special: p@ssword's
      foo: bar
      float: 111.11111111
      int: 111
      MY_NODE_NAME:
        field:
          path: spec.nodeName
          api-version: v1
      my-resource-limit:
        resource:
          container-name: container1
          resource: requests.cpu
          divisor: 1m
      thing:
        secret:
          name: foo
          key: bar
      a-secret:
        secret:
          name: secret1
          optional: true
      another-secret:
        secret:
          name: secret2
      thing1:
        config-map:
          name: foo
          key: bar
      a-configmap:
        config-map:
          name: configmap1
          optional: true
      another-configmap:
        config-map:
          name: configmap2
$ cat reactive/spec_template.yaml
version: 2
omitServiceFrontend: true
containers:
  - name: %(name)s
    imageDetails:
      imagePath: %(docker_image_path)s
      username: %(docker_image_username)s
      password: %(docker_image_password)s
    ports:
      - containerPort: %(port)s
        protocol: TCP
    config:
      MYSQL_ROOT_PASSWORD: %(root_password)s
      MYSQL_USER: %(user)s
      MYSQL_PASSWORD: %(password)s
      MYSQL_DATABASE: %(database)s
      MY_NODE_NAME:
        field:
          path: spec.nodeName
          api-version: v1
      build-robot-secret:
        secret:
          name: build-robot-secret
      ...

$ mkubectl get pod/mariadb-k8s-0 -n t1 -o jsonpath="{..env}"
[map[name:MYSQL_DATABASE value:database] map[name:MYSQL_PASSWORD value:password] map[name:MYSQL_ROOT_PASSWORD value:root] map[name:MYSQL_USER value:admin] map[name:MY_NODE_NAME valueFrom:map[fieldRef:map[apiVersion:v1 fieldPath:spec.nodeName]]] map[name:build-robot-secret valueFrom:map[secretKeyRef:map[key:config.yaml name:build-robot-secret]]]]

$ juju run --unit mariadb-k8s/2 --timeout=30s "env" --operator=false -m k1:t1
MYSQL_ROOT_PASSWORD=root
MYSQL_USER=admin
MYSQL_PASSWORD=password
MYSQL_DATABASE=database
MY_NODE_NAME=ubuntu
username=admin  # from `secret/build-robot-secret`
password=1f2d1e2e67df  # from `secret/build-robot-secret`
...

Documentation changes

Yes

Bug reference

https://bugs.launchpad.net/juju/+bug/1858515

@ycliuhw
Copy link
Member Author

ycliuhw commented Jan 23, 2020

Copy link
Member

@wallyworld wallyworld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to adjust the abstraction used.

Comment on lines 76 to 77
EnvFrom []core.EnvFromSource `json:"envFrom,omitempty" yaml:"envFrom,omitempty"`
Env []core.EnvVar `json:"env,omitempty" yaml:"env,omitempty"`
Copy link
Member

@wallyworld wallyworld Jan 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a slightly different abstraction here.
We already support the "config" keyword for straight pod env key values.

ie

    config:
      attr: foo=bar; name["fred"]="blogs";
      foo: bar

We need to look to extend that to add in the extra capability, eg

    config:
      foo: bar
      my_node-name:
        field:
          name: spec.nodeName
      thing:
         secret:
           name: foo
           key: bar
      thing2:
         secret:
           name: foo
      thing3:
         configMap:
           name: foo
      thing4:
         configMap:
           name: foo
           key: bar

So "config" attributes are extended to accept either a string as we have now for simple scalar values, or a map which is used to define the 3 new cases.
There are 3 new keywords: field, configMap, secret.
If only the name is specified for configMap or secret, that maps to EnvFrom where all fields are used. Otherwise we just use the single named field.

@ycliuhw ycliuhw changed the base branch from 2.7 to develop February 4, 2020 04:39
@ycliuhw ycliuhw force-pushed the feature/pod-value-mount branch 2 times, most recently from 9f55a2f to 1d735f0 Compare February 5, 2020 01:34
@ycliuhw ycliuhw marked this pull request as ready for review February 5, 2020 03:37
Copy link
Member

@wallyworld wallyworld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to add back the special bool handling and remove resource limits

caas/kubernetes/provider/k8s_test.go Show resolved Hide resolved
caas/specs/types.go Outdated Show resolved Hide resolved
caas/kubernetes/provider/specs/v2_test.go Show resolved Hide resolved
caas/kubernetes/provider/specs/types.go Show resolved Hide resolved
@ycliuhw
Copy link
Member Author

ycliuhw commented Feb 5, 2020

$$merge$$

@jujubot jujubot merged commit 5f17025 into juju:develop Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants