-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Remove /var/log symlink logic from the queue proxy #7882
Remove /var/log symlink logic from the queue proxy #7882
Conversation
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.
@dprotaso: 0 warnings.
In response to this:
This can now be accomplished in K8s 1.15 using volume subpath
expressionsA notable side-effect is variables which populated the subpath
expression come from the container's environment.Thus I exposed the pod's name, namespace and container name as
environment variables. Each has a prefixK_INTERNAL_
denoting
this shouldn't be consumed by usersPart of #7881
Release Note
/var/log log capture now supports containers that aren't named `user-container`
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.
/assign @JRBANCEL |
cc @mattmoor as an FYI for the |
This can now be accomplished in K8s 1.15 using volume subpath expressions https://kubernetes.io/docs/concepts/storage/volumes/#using-subpath-with-expanded-environment-variables A notable side-effect is variables which populated the subpath expression come from the container's environment. Thus I exposed the pod's name, namespace and container name as environment variables. Each has a prefix `K_INTERNAL_` denoting this shouldn't be consumed by users
734e0d3
to
6c8c8bf
Compare
Looks very clean! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dprotaso, JRBANCEL 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 |
/retest |
@JRBANCEL: The
Use In response to this:
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. |
/retest all |
@JRBANCEL: The
Use In response to this:
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. |
whoops forgot to delete a test |
The following is the coverage report on the affected files.
|
/hold I should probably add logic reserving |
/hold cancel I'll actually make this a separate PR since it may warrant a discussion |
/lgtm |
The following jobs failed:
Automatically retrying due to test flakiness... |
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'm not qualified to review if this is a non-breaking change but: I love the simplification!
/retest |
@dprotaso: The following test failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
This can now be accomplished in K8s 1.15 using volume subpath
expressions
https://kubernetes.io/docs/concepts/storage/volumes/#using-subpath-with-expanded-environment-variables
A notable side-effect is variables which populated the subpath
expression come from the container's environment.
Thus I exposed the pod's name, namespace and container name as
environment variables. Each has a prefix
K_INTERNAL_
denotingthis shouldn't be consumed by users
Part of #7881
Release Note