-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Adds support for unsetting of env vars #8506
Adds support for unsetting of env vars #8506
Conversation
…of running container-machine Signed-off-by: Harkishen-Singh <[email protected]>
Welcome @Harkishen-Singh! |
Hi @Harkishen-Singh. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Harkishen-Singh The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Can one of the admins verify this patch? |
In reference to #8476 (comment) |
/assign @medyagh |
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.
@Harkishen-Singh thank you for this PR, do you mind putting in the PR description the Before and After this PR's output and explain how it is fixing the issue?
@medyagh sure. |
@medyagh Done! |
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.
@Harkishen-Singh Thank you very much for this helpful PR ! I hope I get to see more contributions from you
@medyagh definitely! Thank you. |
@Harkishen-Singh please feel free to take issues labeled help-wanted or good-first-issue |
@medyagh sure. |
Signed-off-by: Harkishen-Singh [email protected]
Fixes: #8476
Behaviour before this PR
Behaviour after this PR
Reference: #8476 (comment)
Explanation:
The unsettling of env-vars in docker should be independent of whether the docker-machine is running or not. This is minikube specific issue and was occurring due
co := mustload.Running(cname)
. Moving the unset statements up, before the running-checks solves the issue. Similar is the situation for podman.