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

Add openssh-client back into the debian-hyperkube-base image #54250

Merged
merged 1 commit into from
Oct 27, 2017
Merged

Add openssh-client back into the debian-hyperkube-base image #54250

merged 1 commit into from
Oct 27, 2017
+14 −13

Conversation

ixdy
Copy link
Member

@ixdy ixdy commented Oct 19, 2017 •
edited
Loading

What this PR does / why we need it: adds openssh-client back into the debian-hyperkube-base image. This was removed in #48365, but is apparently needed by the gitRepo volume plugin.

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

Special notes for your reviewer:
I haven't yet pushed this image, so builds will fail. If this looks good, I'll push and re-trigger tests.

Release note:

Add openssh-client back into the hyperkube image. This allows the gitRepo volume plugin to work properly.

/assign @luxas @tallclair

@k8s-ci-robot
Copy link
Contributor

@ixdy: Adding do-not-merge/release-note-label-needed because the release note process has not been followed.

One of the following labels is required "release-note", "release-note-action-required", or "release-note-none".
Please see: https://github.com/kubernetes/community/blob/master/contributors/devel/pull-requests.md#write-release-notes-if-needed.

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.

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 19, 2017
ixdy
ixdy commented Oct 19, 2017
View reviewed changes
build/debian-hyperkube-base/Dockerfile
@@ -20,20 +20,21 @@ FROM BASEIMAGE
RUN ln -s /bin/sh /bin/bash

RUN echo CACHEBUST>/dev/null && clean-install \
iptables \
ca-certificates \
Copy link
Member Author

Choose a reason for hiding this comment

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

I sorted this list.

@ixdy
Copy link
Member Author

ixdy commented Oct 19, 2017

/release-note

@k8s-ci-robot
Copy link
Contributor

@ixdy: the /release-note and /release-note-action-required commands have been deprecated.
Please edit the release-note block in the PR body text to include the release note. If the release note requires additional action include the string action required in the release note. For example:

```release-note
Some release note with action required.
```

In response to this:

/release-note

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.

@ixdy ixdy mentioned this pull request Oct 19, 2017
Closed
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. 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. labels Oct 19, 2017
@ixdy
Copy link
Member Author

ixdy commented Oct 19, 2017

#51250 makes the fix for v1.8 a bit more challenging; it should have bumped the debian-hyperkube-base tag, but it didn't, so now debian-hyperkube-base:0.5 will also include cni:0.6, and I'm not sure if we want to backport that to v1.8.

the alternative is that I hack a build of debian-hyperkube-base:0.5 with the old version of CNI, but that becomes hard to reproduce.

tallclair
tallclair reviewed Oct 19, 2017
View reviewed changes
Copy link
Member

@tallclair tallclair left a comment

Choose a reason for hiding this comment

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

lgtm, once updated.

build/root/WORKSPACE Outdated
@@ -70,10 +70,10 @@ docker_pull(

docker_pull(
name = "debian-hyperkube-base-amd64",
digest = "sha256:f3a37c4d8700a5ff454d94a2bef7d165d287759cea737a621c20e4aa3891dbbb",
digest = "sha256:UPDATE",
Copy link
Member

Choose a reason for hiding this comment

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

Will you update this when you push the image? Or is the manual sha no longer needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, I left this here to intentionally break the build, but will fill in once pushed.

cluster/images/hyperkube/Makefile Outdated
@@ -21,7 +21,7 @@ REGISTRY?=gcr.io/google-containers
ARCH?=amd64
HYPERKUBE_BIN?=_output/dockerized/bin/linux/$(ARCH)/hyperkube

BASEIMAGE=gcr.io/google-containers/debian-hyperkube-base-$(ARCH):0.4
BASEIMAGE=gcr.io/google-containers/debian-hyperkube-base-$(ARCH):0.5
Copy link
Member

Choose a reason for hiding this comment

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

FYI - will conflict with #52744

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, I already reached out to @rphillips to sequence

@ixdy
Copy link
Member Author

ixdy commented Oct 19, 2017

better solution, I guess: I will build debian-hyperkube-base:0.5 as it exists in master (with cni 0.6), but will change the tag to 0.4.1 when cherrypicking this to release-1.8 (without cni updates).

@ixdy
Copy link
Member Author

ixdy commented Oct 19, 2017

The s390x and ppc64le CNI plugins tarballs are missing, so I can't build the new debian-hyperkube-base images yet.

@imkin
Copy link

imkin commented Oct 20, 2017

@ixdy do not understand what would help getting the cni plugins tarballs. Is there another PR we are dependent on?

@ixdy ixdy force-pushed the debian-hyperkube-base-ssh branch from 2349bf7 to aa82253 Compare October 20, 2017 21:45
@ixdy
Copy link
Member Author

ixdy commented Oct 20, 2017

still not pushed; waiting on #54272 to bump tag for the CNI version bump.

@ixdy ixdy mentioned this pull request Oct 20, 2017
Merged
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 22, 2017
@luxas
Copy link
Member

luxas commented Oct 22, 2017

#54272 was merged

@ixdy ixdy force-pushed the debian-hyperkube-base-ssh branch from aa82253 to e7084dc Compare October 23, 2017 07:29
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 23, 2017
@ixdy ixdy force-pushed the debian-hyperkube-base-ssh branch from e7084dc to 9658820 Compare October 23, 2017 07:30
@ixdy
Copy link
Member Author

ixdy commented Oct 23, 2017

PR rebased and images pushed.

@ixdy
Copy link
Member Author

ixdy commented Oct 26, 2017

@mikedanese @roberthbailey @eparis can one of you approve?

@ixdy ixdy force-pushed the debian-hyperkube-base-ssh branch from 9658820 to 71624d8 Compare October 26, 2017 22:03
@roberthbailey
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 26, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ixdy, roberthbailey

Associated issue: 48365

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 26, 2017
@ixdy
Copy link
Member Author

ixdy commented Oct 27, 2017

/test all

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 54635, 54250, 54657, 54696, 54700). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 08ce10e into kubernetes:master Oct 27, 2017
k8s-github-robot pushed a commit that referenced this pull request Nov 2, 2017
Merge pull request #54325 from ixdy/automated-cherry-pick-of-#53642-#…
0ce7d53 
…54250-upstream-release-1.8

Automatic merge from submit-queue.

Automated cherry pick of #54250

Cherry pick of #54250 on release-1.8.

#54250: Add openssh-client to the debian-hyperkube-base image

```release-note
Adds modprobe and openssh-client back into the hyperkube image. 
```
@ixdy ixdy deleted the debian-hyperkube-base-ssh branch May 15, 2018 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tallclair tallclair tallclair left review comments

Assignees

@luxas luxas

@roberthbailey roberthbailey

@tallclair tallclair

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. 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/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Creation of gitRepo volume is broken in 1.8.0+
7 participants
@ixdy @k8s-ci-robot @imkin @luxas @roberthbailey @k8s-github-robot @tallclair