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.

Sign up for GitHub

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

Jump to bottom

Enable k8s configmaps as flags for play kube #7910

Merged
merged 1 commit into from
Oct 9, 2020
Merged

Enable k8s configmaps as flags for play kube #7910

merged 1 commit into from
Oct 9, 2020

Conversation

EduardoVega
Copy link
Contributor

@EduardoVega EduardoVega commented Oct 3, 2020 •
edited
Loading

This will permit podman play kube to obtain container env vars from configmaps. By passing the --configmap flag with a valid configmap.yaml, it will search for the pod.container.env and pod.container.envFrom settings to validate if a configmap has been conigured as the source.

Examples:
podman play kube [pod|deploy].yaml --configmap myconfigmap.yaml
podman play kube [pod|deploy].yaml --configmap myconfigmap1.yaml --configmap myconfigmap2.yaml

Fixes: #7567

Signed-off-by: Eduardo Vega edvegavalerio@gmail.com

@EduardoVega EduardoVega force-pushed the 7567-podman-configmaps branch from 58fc897 to 1b96ba4 Compare October 3, 2020 18:45
@rhatdan
Copy link
Member

rhatdan commented Oct 4, 2020

Thanks @EduardoVega
Nice work on all of the tests.
@haircommander @vrothberg @mheon PTAL

@rhatdan
Copy link
Member

rhatdan commented Oct 4, 2020

@saschagrunert @zhangguanzhang PTAL

@rhatdan
Copy link
Member

rhatdan commented Oct 5, 2020

/approve

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 5, 2020
mheon
mheon reviewed Oct 5, 2020
View reviewed changes
pkg/domain/infra/abi/play.go Outdated Show resolved Hide resolved
saschagrunert
saschagrunert approved these changes Oct 5, 2020
View reviewed changes
Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

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

Just a nit, otherwise LGTM

docs/source/markdown/podman-play-kube.1.md Outdated Show resolved Hide resolved
@rhatdan
Copy link
Member

rhatdan commented Oct 5, 2020

Please rebase also for new CI.

@TomSweeneyRedHat
Copy link
Member

@EduardoVega very nice PR overall, TY for putting it together!

@rhatdan
Copy link
Member

rhatdan commented Oct 5, 2020

This will need a rebase to pass the tests.

@EduardoVega
Copy link
Contributor Author

Thanks. I'll work on the changes and rebase.

@haircommander
Copy link
Collaborator

a couple of naming suggestions, otherwise LGTM, thanks @EduardoVega

@EduardoVega EduardoVega force-pushed the 7567-podman-configmaps branch 2 times, most recently from 858a4ae to 104b5bf Compare October 7, 2020 13:23
@EduardoVega
Enable k8s configmaps as flags for play kube
39dde9b 
Signed-off-by: Eduardo Vega <edvegavalerio@gmail.com>
@EduardoVega EduardoVega force-pushed the 7567-podman-configmaps branch from 104b5bf to 39dde9b Compare October 7, 2020 14:55
@EduardoVega
Copy link
Contributor Author

This change does not support podman-remote. I can create another bug to work on that, if it is wanted.

@rhatdan
Copy link
Member

rhatdan commented Oct 7, 2020

SGTM
Thanks @EduardoVega
/lgtm
/approve
/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 7, 2020
@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 7, 2020
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: EduardoVega, rhatdan, saschagrunert

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

@rhatdan
Copy link
Member

rhatdan commented Oct 9, 2020

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 9, 2020
@openshift-merge-robot openshift-merge-robot merged commit 953e16f into containers:master Oct 9, 2020
@edsantiago edsantiago mentioned this pull request Oct 19, 2020
Closed
@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 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mheon mheon mheon left review comments

@haircommander haircommander haircommander left review comments

@zhangguanzhang zhangguanzhang zhangguanzhang left review comments

@TomSweeneyRedHat TomSweeneyRedHat TomSweeneyRedHat left review comments

@saschagrunert saschagrunert saschagrunert approved these changes

Assignees

@rhatdan rhatdan

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Kubernetes ConfigMaps in podman
9 participants
@EduardoVega @rhatdan @TomSweeneyRedHat @haircommander @openshift-ci-robot @saschagrunert @mheon @zhangguanzhang @openshift-merge-robot