-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 Deployment generation with kube generate #17950
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: umohnani8 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 |
Deployment generation examples. For a pod:
|
All kinds of test unhappiness @umohnani8 |
a06505e
to
678b71b
Compare
Tests are mostly green! @containers/podman-maintainers PTAL |
Will review tomorrow morning. @containers/podman-maintainers PTAL |
a115e75
to
ece9e86
Compare
The podman kube generate command can now generate a Deployment kind when the --ype flag is set to deployment. By default, a Pod spec will be generated if --type flag is not set. Add --replicas flag to kube generate to allow users to set the value of replicas in the generated yaml when generating a Deployment kind. Add e2e and minikube tests for this feature. Signed-off-by: Urvashi Mohnani <[email protected]>
LGTM, thanks for getting this out so fast :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Let's wait for a head nod from @edsantiago for the tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know enough about minikube to review the e2e or minikube tests. One question in system tests, otherwise LGTM
@@ -106,24 +106,23 @@ metadata.labels.app | = | ${pname} | |||
metadata.name | = | ${pname} | |||
|
|||
spec.hostname | = | $pname | |||
spec.restartPolicy | = | Never |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any particular reason not to change this one from Never
to null
, like the others? Why are you removing this check entirely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
restartPolicy will not be a field in the generated yaml for the default case. I don't think null would be a valid value here as it isn't a pointer as the others are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can open another PR to test that out, but would like to get this one in today before we need to cut the release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries, thanks for the explanation. LGTM then.
This is ready, please merge when possible @mheon @rhatdan @vrothberg @baude |
/lgtm |
The podman kube generate command can now generate a
Deployment kind when the --type flag is set to deployment.
By default, a Pod spec will be generated if --type flag is
not set.
Add --replicas flag to kube generate to allow users to set
the value of replicas in the generated yaml when generating a
Deployment kind.
Add e2e and minikube tests for this feature.
Fixes #17712
Signed-off-by: Urvashi Mohnani [email protected]
Does this PR introduce a user-facing change?