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

Dynamic env in subpath - Fixes Issue 48677 #49388

Merged
merged 1 commit into from
May 30, 2018
Merged

Dynamic env in subpath - Fixes Issue 48677 #49388

merged 1 commit into from
May 30, 2018

Conversation

kevtaylor
Copy link
Contributor

@kevtaylor kevtaylor commented Jul 21, 2017 •
edited
Loading

What this PR does / why we need it:

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #48677

Special notes for your reviewer:

Release note:

Adds the VolumeSubpathEnvExpansion alpha feature to support environment variable expansion
Sub-paths cannot be mounted with a dynamic volume mount name.
This fix provides environment variable expansion to sub paths
This reduces the need to manage symbolic linking within sidecar init containers to achieve the same goal

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 21, 2017
@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Jul 21, 2017
@mtaufen
Copy link
Contributor

mtaufen commented Jul 21, 2017

I commented on #48677. Help me understand the value of this addition.

@kevtaylor
Copy link
Contributor Author

You cannot at present use a dynamic env var in subpath. This allows uniqueness of a folder particularly when mutiple pods write the same output such as a log file. You can only achieve the same results with an init container

@kevtaylor
Copy link
Contributor Author

/assign @ixdy @vishh

@redbaron
Copy link
Contributor

/sig node

@k8s-ci-robot k8s-ci-robot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Jul 25, 2017
@ixdy
Copy link
Member

ixdy commented Jul 25, 2017

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 25, 2017
@kevtaylor
Copy link
Contributor Author

/retest

@kevtaylor
Copy link
Contributor Author

@mtaufen Please can you give us an update on this PR?

@kevtaylor
Copy link
Contributor Author

@ixdy Please can we have an update on this PR?

@mtaufen
Copy link
Contributor

mtaufen commented Aug 8, 2017

We need to resolve the discussion in #48677. At the very least you need to engage more people in discussing this API change. Start with the ones I @-mentioned on that issue.

@kevtaylor
Copy link
Contributor Author

@thockin @lavalamp @smarterclayton @devin-donnelly Please can you have a look at this PR and the related discussion and provide some feedback as to whether it is good to proceed?

@k8s-github-robot k8s-github-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. release-note Denotes a PR that will be considered when it comes time to generate release notes. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. release-note-label-needed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 30, 2017
@kevtaylor
Copy link
Contributor Author

/retest

@kevtaylor
Copy link
Contributor Author

/test pull-kubernetes-e2e-kops-aws

@kevtaylor
Copy link
Contributor Author

/test pull-kubernetes-e2e-gce-etcd3

@kevtaylor
Copy link
Contributor Author

/test pull-kubernetes-e2e-kops-aws

@kevtaylor
Copy link
Contributor Author

/test pull-kubernetes-e2e-gce-etcd3

@kevtaylor
Copy link
Contributor Author

/retest

@kevtaylor
Copy link
Contributor Author

/test pull-kubernetes-e2e-gce

@kevtaylor kevtaylor force-pushed the feature/Dynamic-env-in-subpath branch from 00fe053 to fbb6747 Compare May 29, 2018 14:02
@kevtaylor
Copy link
Contributor Author

/test pull-kubernetes-e2e-kops-aws

@kevtaylor
Copy link
Contributor Author

/test pull-kubernetes-integration

@kevtaylor kevtaylor force-pushed the feature/Dynamic-env-in-subpath branch from fbb6747 to b2d4426 Compare May 29, 2018 16:04
@msau42
Copy link
Member

msau42 commented May 29, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 29, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kevtaylor, msau42, saad-ali, yujuhong

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

@kevtaylor
Copy link
Contributor Author

/retest

@fejta
Copy link
Contributor

fejta commented May 29, 2018

fejta , this is some serious spamming going on here

And yet not enough to motivate someone to type /lgtm cancel on the broken PR 😛

@kevtaylor
Copy link
Contributor Author

/retest

1 similar comment
@kevtaylor
Copy link
Contributor Author

/retest

@kevtaylor
Copy link
Contributor Author

@saad-ali Please add milestone

@jberkus
Copy link

jberkus commented May 30, 2018

Has there been a security review of this PR? I'm concerned that it was written before our security subpath fixes in March.

@jberkus
Copy link

jberkus commented May 30, 2018

Based on discussion from SIG-Storage channel on Slack:

As such, adding milestone.

/milestone v1.11

@k8s-ci-robot k8s-ci-robot added this to the v1.11 milestone May 30, 2018
@jberkus
Copy link

jberkus commented May 30, 2018

/priority important-soon

@k8s-ci-robot k8s-ci-robot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label May 30, 2018
@k8s-github-robot
Copy link

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@jsafrane @kevtaylor @msau42 @mtaufen @smarterclayton

Pull Request Labels
  • sig/node sig/storage: Pull Request will be escalated to these SIGs if needed.
  • priority/important-soon: Escalate to the pull request owners and SIG owner; move out of milestone after several unsuccessful escalation attempts.
  • kind/feature: New functionality.
Help

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 58920, 58327, 60577, 49388, 62306). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 6b2fc7c into kubernetes:master May 30, 2018
@kevtaylor kevtaylor mentioned this pull request Jun 1, 2018
Closed
@fatsheep9146 fatsheep9146 mentioned this pull request Jul 19, 2018
Merged
@kevtaylor kevtaylor mentioned this pull request Nov 22, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yujuhong yujuhong yujuhong left review comments

@msau42 msau42 msau42 left review comments

@saad-ali saad-ali Awaiting requested review from saad-ali

Assignees

@mtaufen mtaufen

@smarterclayton smarterclayton

@jsafrane jsafrane

@msau42 msau42

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.11
Development

Successfully merging this pull request may close these issues.

Downward API support in volume subPath