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

rkt: Convert image name to be a valid acidentifier #34375

Merged
merged 1 commit into from
Oct 20, 2016
Merged

rkt: Convert image name to be a valid acidentifier #34375

merged 1 commit into from
Oct 20, 2016

Conversation

euank
Copy link
Contributor

@euank euank commented Oct 8, 2016 •
edited
Loading

Release note:

Fix a bug under the rkt runtime whereby image-registries with ports would not be fetched from

This fixes a bug whereby an image reference that included a port was not
recognized after being downloaded, and so could not be run

This is the quick-and-simple fix. In the longer term, we'll want to refactor image logic a bit more to handle the many special cases that the current code does not, mostly related to library images on dockerhub.

/cc @yifan-gu @kubernetes/sig-rktnetes

This change is Reviewable

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Oct 8, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins Kubemark GCE e2e failed for commit 7077378. Full PR test history.

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.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GCE e2e failed for commit 7077378. Full PR test history.

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.

@euank
Copy link
Contributor Author

euank commented Oct 10, 2016

@k8s-bot kubemark e2e test this
@k8s-bot gci gce e2e test this

yifan-gu
yifan-gu reviewed Oct 11, 2016
View reviewed changes
pkg/kubelet/rkt/image.go
},
}

// If the repo name is not a valid ACIdentifier (namely if it has a portt),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit s/portt/port

@yifan-gu
Copy link
Contributor

one small nit, LGTM.

@yifan-gu yifan-gu added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Oct 11, 2016
@euank
rkt: Convert image name to be a valid acidentifier
aff6940 
This fixes a bug whereby an image reference that included a port was not
recognized after being downloaded, and so could not be run
@euank
Copy link
Contributor Author

euank commented Oct 11, 2016 •
edited
Loading

Addressed your comment. This seems like a release-note type thing since it's a user-visible bugfix.

@yifan-gu yifan-gu added release-note Denotes a PR that will be considered when it comes time to generate release notes. lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Oct 11, 2016
@yifan-gu
Copy link
Contributor

@k8s-bot test this please issue #IGNORE

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE e2e failed for commit aff6940. Full PR test history.

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

@euank
Copy link
Contributor Author

euank commented Oct 11, 2016

Failure is:

 - Quota 'FIREWALLS' exceeded. Limit: 250.0

@k8s-bot gce e2e test this: issue #33867

@yifan-gu
Copy link
Contributor

@k8s-bot gce e2e test this: issue #33867

@yifan-gu
Copy link
Contributor

@k8s-bot test his please issue #IGNORE

@euank
Copy link
Contributor Author

euank commented Oct 20, 2016 •
edited
Loading

@k8s-bot gce e2e test this

(Error queueing build, no details on why)

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit c04aec3 into kubernetes:master Oct 20, 2016
@euank euank deleted the rkt-image-with-port branch October 20, 2016 22:02
@euank
Copy link
Contributor Author

euank commented Nov 8, 2016

Someone impacted by this bug has asked if it can be cherry picked onto 1.4.

I'm happy to make the rebase/PR.

IIUC first I should ask to have a cherrypick-candidate label added here though. Mind applying it?

cc @saad-ali @calebamiles

@saad-ali
Copy link
Member

saad-ali commented Nov 9, 2016

CC @jessfraz

@saad-ali saad-ali added this to the v1.4 milestone Nov 9, 2016
@jessfraz jessfraz added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Nov 9, 2016
@jessfraz jessfraz mentioned this pull request Nov 9, 2016
Merged
@jessfraz
Copy link
Contributor

jessfraz commented Nov 9, 2016

cherry-picked in #36510

jessfraz added a commit that referenced this pull request Nov 9, 2016
@jessfraz
Merge pull request #36510 from jessfraz/automated-cherry-pick-of-#34375-
d72ad89 
#35285-#35420-#35608-#35919-#36161-origin-release-1.4

Automated cherry pick of #34375 #35285 #35420 #35608 #35919 #36161
shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016
@jessfraz
Merge pull request kubernetes#36510 from jessfraz/automated-cherry-pi…
875d3fe 
…ck-of-#34375-kubernetes#35285-kubernetes#35420-kubernetes#35608-kubernetes#35919-kubernetes#36161-origin-release-1.4

Automated cherry pick of kubernetes#34375 kubernetes#35285 kubernetes#35420 kubernetes#35608 kubernetes#35919 kubernetes#36161
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yifan-gu yifan-gu yifan-gu left review comments

Assignees

@yifan-gu yifan-gu

Labels
cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. 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/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.4
Development

Successfully merging this pull request may close these issues.
7 participants
@euank @k8s-ci-robot @yifan-gu @k8s-github-robot @saad-ali @jessfraz @googlebot