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]: use valueFrom in container type env section #1703

Closed
wojtkowiak opened this issue Mar 16, 2020 · 3 comments · Fixed by #1742
Closed

[FEATURE]: use valueFrom in container type env section #1703

wojtkowiak opened this issue Mar 16, 2020 · 3 comments · Fixed by #1742

Comments

@wojtkowiak
Copy link

wojtkowiak commented Mar 16, 2020

Feature Request

Would it be possible to use the valueFrom the same way secretRef is currently supported?

valueFrom:
  fieldRef:
    fieldPath: spec.nodeName

Background / Motivation

So I am just starting with garden and I may be very wrong here but because I need the nodeName passed into an env I will not be able to use the simplicity of type: container and will have to still keep my stuff in kubernetes yml which is a bummer 🙂
I actually spent like 40 minutes wondering why it is not working because seeing the secretRef being used in an example I just conjectured I can use any other directives I would normally be able to use in k8 yml

What should the user be able to do?

services:
  - env:
        - MY_VAR: some-value
          MY_OTHER_VAR:
            valueFrom:
              fieldRef:
                fieldPath: spec.nodeName

Why do they want to do this? What problem does it solve?

described in motivation section above

Suggested Implementation(s)

described above

How important is this feature for you/your team?

🌵 Not having this feature makes using Garden painful - exaggerating here but it ruined the excitement for a moment 🙂

@wojtkowiak wojtkowiak changed the title [FEATURE]: use valueFrom in container type env [FEATURE]: use valueFrom in container type env section Mar 16, 2020
@eysi09
Copy link
Collaborator

eysi09 commented Mar 18, 2020

Hi @wojtkowiak and thanks for opening this issue!

The container module type isn't really aware of Kubernetes and could conceptually be deployed by other providers as well. That's why we're vary of adding Kubernetes specific fields that don't translate to other potential platforms.

I'm assuming the node names are dynamic and you simply don't want to go for MY_OTHER_VAR: my-node-name.

If you're using Terraform to provision your infrastructure, you could read the value from the Terraform outputs. Something like:

MY_OTHER_VAR: "${runtime.services.my-terraform-service.outputs.node-name}"

Another solution would be to use a task that could read the node name and reference that at the service level. Like:

services:
  - env:
        - MY_VAR: some-value
          MY_OTHER_VAR: "${runtime.tasks.read-node-name.outputs.log}"
tasks:
  - name: read-node-name
     command:  # Add command...
     args: # ...and args for reading the nodename

And finally, if you want to run the task locally as opposed to from within the cluster, you can wrap it in a local exec module:

services:
  - env:
        - MY_VAR: some-value
          MY_OTHER_VAR: "${runtime.tasks.read-node-name.outputs.log}"
# Rest of module config...

---

kind: Module
name: node-name-reader
type: exec
local: true
tasks:
  - name: read-node-name
     command:  ["kubectl", "get", "nodes", "..."] <--- This runs locally

Would any of these help your use case?

@edvald
Copy link
Collaborator

edvald commented Mar 27, 2020

There is another idea... We can include some of these common metadata fields automatically when generating the Deployment from container modules. I'll push a PR for that shortly, which among others sets POD_NODE_NAME to the spec.nodeName value. @wojtkowiak any other values that you could imagine being useful?

edvald added a commit that referenced this issue Mar 27, 2020
Specifically adding POD_HOST_IP, POD_UID and POD_NODE_NAME. This is
much simpler than adding `valueFrom` to the container module spec, and
I think it's fine to add any standard/common fields like this in our
code.

Fixes #1703 (partially at least)
edvald added a commit that referenced this issue Mar 27, 2020
Specifically adding POD_HOST_IP, POD_UID and POD_NODE_NAME. This is
much simpler than adding `valueFrom` to the container module spec, and
I think it's fine to add any standard/common fields like this in our
code.

Fixes #1703 (partially at least)
edvald added a commit that referenced this issue Mar 29, 2020
Specifically adding POD_HOST_IP, POD_UID and POD_NODE_NAME. This is
much simpler than adding `valueFrom` to the container module spec, and
I think it's fine to add any standard/common fields like this in our
code.

Fixes #1703 (partially at least)
@wojtkowiak
Copy link
Author

@eysi09 sorry for late response, I was going to try your ideas this week but seems that @edvald solved my problem. Anyway your ideas gave me a hint to solve another problem I had so thanks anyway!

@edvald thank you very much, your PR covers all I ever needed!

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

Successfully merging a pull request may close this issue.

3 participants