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

Add support for adding custom labels and environment variables to nodes via spec #149

Merged

Conversation

jlamillan
Copy link
Contributor

@jlamillan jlamillan commented May 21, 2022

Greetings!

This pull-request resolves #135 by adding support add custom labels and environment variables to nodes via spec, which get propagated to the node deployments/pods.

  nodePools:
    - component: nodes
      ...
      labels:
         myLabel: foo
         anotherLabel: bar
      env:
        - name: qux
          value: "quux"
        - name: quuz
          valueFrom:
            configMapKeyRef:
              name: quuz-configmap
              key: corge

@segalziv segalziv requested a review from idanl21 May 21, 2022 05:12
@jlamillan jlamillan force-pushed the jlamillan/add_labels_envvar_opts branch 2 times, most recently from 4f61f6e to 93d3b9a Compare May 23, 2022 23:11
@jlamillan
Copy link
Contributor Author

I also added test coverage for user defined labels and env vars.

@swoehrl-mw
Copy link
Collaborator

Two comments from my side:

  • I would prefer the field to be named just env instead of envVars, that way we would be consistent with kubernetes naming
  • We should consider making it more generic so that users can specify the same things as they can in a pod spec (so not just value but also valueFrom)

@jlamillan
Copy link
Contributor Author

Two comments from my side:

* I would prefer the field to be named just `env` instead of `envVars`, that way we would be consistent with kubernetes naming

* We should consider making it more generic so that users can specify the same things as they can in a pod spec (so not just value but also valueFrom)

Sounds good. I've made these changes and squashed the commits.

@swoehrl-mw
Copy link
Collaborator

LGTM. If you could work in the complaint from the linter, I'll approve the PR.

…les on nodes.

Signed-off-by: jesse.millan <jesse.millan@oracle.com>
@jlamillan jlamillan force-pushed the jlamillan/add_labels_envvar_opts branch from dd00563 to 52a4330 Compare May 25, 2022 20:36
@jlamillan
Copy link
Contributor Author

LGTM. If you could work in the complaint from the linter, I'll approve the PR.

All set.

@swoehrl-mw swoehrl-mw merged commit 0c39a9b into opensearch-project:main May 26, 2022
@swoehrl-mw
Copy link
Collaborator

@jlamillan Merged. Thank you for your work with this PR. Much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How do I add additional labels and additional Env variables for mater nodes
5 participants