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.

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

azure disk: restrict length of name #40030

Conversation

colemickens
Copy link
Contributor

What this PR does / why we need it:
Fixes dynamic disk provisioning on Azure by properly truncating the disk name to conform to the Azure API spec.

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

Special notes for your reviewer:
n/a

Release note:

azure disk: restrict name length for Azure specifications

cc: @rootfs

@k8s-reviewable
Copy link

This change is Reviewable

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 17, 2017
@k8s-ci-robot
Copy link
Contributor

Jenkins Bazel Build failed for commit 34d1811d1c39e063a75d1f3b7258d99067636b12. Full PR test history. cc @colemickens

The magic incantation to run this job again is @k8s-bot bazel test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

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.

@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jan 17, 2017
@k8s-ci-robot
Copy link
Contributor

Jenkins Kubemark GCE e2e failed for commit 34d1811d1c39e063a75d1f3b7258d99067636b12. Full PR test history. cc @colemickens

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

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.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE Node e2e failed for commit 34d1811d1c39e063a75d1f3b7258d99067636b12. Full PR test history. cc @colemickens

The magic incantation to run this job again is @k8s-bot node e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

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.

@k8s-ci-robot
Copy link
Contributor

Jenkins kops AWS e2e failed for commit 34d1811d1c39e063a75d1f3b7258d99067636b12. Full PR test history. cc @colemickens

The magic incantation to run this job again is @k8s-bot kops aws e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

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.

@k8s-ci-robot
Copy link
Contributor

Jenkins GKE smoke e2e failed for commit 34d1811d1c39e063a75d1f3b7258d99067636b12. Full PR test history. cc @colemickens

The magic incantation to run this job again is @k8s-bot cvm gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

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.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GKE smoke e2e failed for commit 34d1811d1c39e063a75d1f3b7258d99067636b12. Full PR test history. cc @colemickens

The magic incantation to run this job again is @k8s-bot gci gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

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.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE e2e failed for commit 34d1811d1c39e063a75d1f3b7258d99067636b12. Full PR test history. cc @colemickens

The magic incantation to run this job again is @k8s-bot cvm gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

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.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE etcd3 e2e failed for commit 34d1811d1c39e063a75d1f3b7258d99067636b12. Full PR test history. cc @colemickens

The magic incantation to run this job again is @k8s-bot gce etcd3 e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

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.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GCE e2e failed for commit 34d1811d1c39e063a75d1f3b7258d99067636b12. Full PR test history. cc @colemickens

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

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.

@k8s-ci-robot
Copy link
Contributor

Jenkins unit/integration failed for commit 34d1811d1c39e063a75d1f3b7258d99067636b12. Full PR test history. cc @colemickens

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

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.

@k8s-ci-robot
Copy link
Contributor

Jenkins CRI GCE Node e2e failed for commit 34d1811d1c39e063a75d1f3b7258d99067636b12. Full PR test history. cc @colemickens

The magic incantation to run this job again is @k8s-bot cri node e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

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.

@colemickens colemickens force-pushed the colemickens-dyn-disk-name-length branch from 34d1811 to 8adcf07 Compare January 17, 2017 17:39
@rootfs
Copy link
Contributor

rootfs commented Jan 17, 2017

LGTM

@k8s-ci-robot
Copy link
Contributor

Jenkins verification failed for commit 34d1811d1c39e063a75d1f3b7258d99067636b12. Full PR test history. cc @colemickens

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

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.

@brendandburns
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 18, 2017
@brendandburns
Copy link
Contributor

fwiw, we should consider adding an admission controller to test for this. If two names happen to truncate to the same first 75 letters it's going to give some pretty hard to track errors.

@colemickens
Copy link
Contributor Author

(Not that I disagree, but I don't think this changes increases chances of collision though. The thing that winds up truncated here is the prefix, and it's the same for the entire cluster anyway.)

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 39826, 40030)

@k8s-github-robot k8s-github-robot merged commit 8f99b74 into kubernetes:master Jan 18, 2017
@colemickens
Copy link
Contributor Author

@saad-ali Are you in charge of cherry-picks for 1.5.x? Can this be marked as a cherry-pick?

@codablock
Copy link
Contributor

@colemickens Which behavior/bug does this PR fix? Complete failure to mount or something related to the time needed for attach/deatch? I'm asking because I may also want to cherry-pick this into my custom built branch for our deployments.

@colemickens
Copy link
Contributor Author

@codablock Without this patch, if the clustername is too large, the PV created will have a Name that is longer than 79 character. The attach operation fails repeatedly in this case as the PUT is rejected.

@colemickens
Copy link
Contributor Author

ping @saad-ali for a 1.5.x cherry-pick.

@colemickens colemickens deleted the colemickens-dyn-disk-name-length branch February 3, 2017 21:07
@saad-ali saad-ali added this to the v1.5 milestone Feb 10, 2017
@saad-ali saad-ali added cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. cherrypick-candidate labels Feb 10, 2017
k8s-github-robot pushed a commit that referenced this pull request Feb 10, 2017
…40030-kubernetes-release-1.5

Automatic merge from submit-queue

Automated cherry pick of #40030

Cherry pick of #40030 on release-1.5.

#40030: azure disk: restrict length of name
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.5" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants