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 #12447

Merged

Conversation

jakub-dzon
Copy link
Contributor

What this PR does / why we need it:

This PR is a back port of #12371.

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.

Which issue(s) this PR fixes:

Fixes #12363

@rhatdan
Copy link
Member

rhatdan commented Nov 30, 2021

LGTM

})

It("podman play kube uses all env values from both sources", func() {
SkipIfRemote("--configmaps is not supported for remote")
Copy link
Member

Choose a reason for hiding this comment

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

I know this is a backport but why is this skipped for remote, I thought this PR should fix the remote client so it should be tested in CI as remote.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because this particular test uses both new ConfigMap-in-payload approach that works in remote setup and old --configmaps parameter that does not.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not a blocker: Could we convert --configmap from static file to payload and send it over ? But following change should go in main as well since it was not supported in original PR.

@flouthoc
Copy link
Collaborator

LGTM

@rhatdan
Copy link
Member

rhatdan commented Nov 30, 2021

/approve
/lgtm
Lets continue discussion on ConfigMaps in the main branch not in this backport.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 30, 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 lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 30, 2021
@openshift-merge-robot openshift-merge-robot merged commit 67d5b21 into containers:v3.4 Nov 30, 2021
@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.

5 participants