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

Support env variables based on ConfigMaps sent in payload #12371

Merged
merged 1 commit into from
Nov 23, 2021

Conversation

jakub-dzon
Copy link
Contributor

@jakub-dzon jakub-dzon commented Nov 19, 2021

What this PR does / why we need it:

This PR adds support for providing ConfigMap YAML definitions as part of podman play kube input. ConfigMap provided this way can be referred to as a source of environments variables for pods.

So far it has only been possible to use ConfigMaps defined in files local to podman starting the pods using --configmaps parameter, which does not work in remote scenarios (even if the HTTP call is made withing the same host).
This PR aims at dealing with the same problem as the other attempt in #12243.

How to verify it

Add ConfigMap definition to a YAML multidoc and refer to it in a container defined in a pod in the same file; properties defined in the ConfigMap will be visible in the pod as environment variables. See https://github.com/containers/podman/pull/12371/files#diff-cef3dd63f572aaef036dce65d94140edbfa8c350c2fc836e864a6b1ef653db87R2952 or https://github.com/containers/podman/pull/12371/files#diff-cef3dd63f572aaef036dce65d94140edbfa8c350c2fc836e864a6b1ef653db87R3022

Which issue(s) this PR fixes:

Fixes #12363

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 19, 2021
@jakub-dzon jakub-dzon force-pushed the env-config-maps branch 4 times, most recently from 56fe968 to 4f46c9b Compare November 22, 2021 17:29
@jakub-dzon jakub-dzon changed the title [WIP] Support env variables based on ConfigMaps sent in payload Support env variables based on ConfigMaps sent in payload Nov 22, 2021
@jakub-dzon jakub-dzon marked this pull request as ready for review November 22, 2021 17:29
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 22, 2021
@rhatdan
Copy link
Member

rhatdan commented Nov 22, 2021

@umohnani8 PTAL

@rhatdan
Copy link
Member

rhatdan commented Nov 22, 2021

Thanks @jakub-dzon But tests are not happy.

@jakub-dzon
Copy link
Contributor Author

Looks like some env issue:

# cd .; git clone -- https://go.googlesource.com/tools /var/tmp/go/src/golang.org/x/tools
Cloning into '/var/tmp/go/src/golang.org/x/tools'...
error: 14768 bytes of body are still expected
fetch-pack: unexpected disconnect while reading sideband packet
fatal: early EOF
fatal: fetch-pack: invalid index-pack output
package golang.org/x/tools/cmd/goimports: exit status 128

Rebasing.

@rhatdan
Copy link
Member

rhatdan commented Nov 23, 2021

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 23, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jakub-dzon, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 23, 2021
@umohnani8
Copy link
Member

umohnani8 commented Nov 23, 2021

LGTM

@jakub-dzon
Copy link
Contributor Author

One questions, otherwise LGTM

Thank you!

@rhatdan
Copy link
Member

rhatdan commented Nov 23, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 23, 2021
@openshift-merge-robot openshift-merge-robot merged commit 1be4c36 into containers:main Nov 23, 2021
@jakub-dzon
Copy link
Contributor Author

@rhatdan: would you agree to backporting this feature to the 3.x line so we can consume it sooner than v4 would be released?
There are no breaking changes introduced by this PR and it should be fairly simple to backport.

@rhatdan
Copy link
Member

rhatdan commented Nov 29, 2021

Sure open a Back port PR and we can get it merged and into the next bugfix release.

@jakub-dzon
Copy link
Contributor Author

Sure open a Back port PR and we can get it merged and into the next bugfix release.

Thank you! I've created #12447.

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add end-to-end support for config map-based env variables in play kube
4 participants