Do not fake /bin/bash, just use the real bash #55018

dims · 2017-11-02T18:14:01Z

What this PR does / why we need it:

Looks like 0.6, we ended up with dash as the default shell, with
/bin/sh as well as /bin/dash ending up invoking dash.

We should not change the contract by faking a link to /bin/bash.
Let's install the actual bash package and make sure /bin/sh is
linked to /bin/bash as well.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #55012

Special notes for your reviewer:

Release note:

Fix problem with /bin/bash ending up linked to dash

ixdy · 2017-11-02T18:17:18Z

I guess this is fine for hyperkube, which already has a huge number of additional packages. Do you feel similarly, @tallclair?

ixdy · 2017-11-02T18:18:12Z

build/debian-hyperkube-base/Dockerfile

-# The samba-common, cifs-utils, and nfs-common packages depend on
-# ucf, which itself depends on /bin/bash existing.
-# It doesn't seem to actually need bash, however.
-RUN ln -s /bin/sh /bin/bash


I guess we could also remove the symlink after installing the packages, though that's a pretty ugly hack, too.

Right @ixdy

tallclair · 2017-11-02T20:24:04Z

~~It means more maintenance burden on us to keep the packages up-to-date, but if it's required it's required...~~

EDIT: actually, we rebuild hyperkube with every release, so this isn't much of a burden. I'm fine with this.

dims · 2017-11-16T15:58:58Z

/assign @luxas

ixdy · 2017-11-16T19:05:10Z

build/debian-hyperkube-base/Dockerfile

@@ -38,3 +34,8 @@ RUN echo CACHEBUST>/dev/null && clean-install \
    util-linux

 COPY cni-bin/bin /opt/cni/bin
+
+# The samba-common, cifs-utils, and nfs-common packages depend on


I think you need to do this before the clean-install step, as otherwise the package configuration step fails.

Looks like i have to install bash separately too before installing the rest. will rework

dims · 2017-11-16T19:17:19Z

/hold

dims · 2017-11-16T22:35:21Z

/hold cancel

dims · 2017-11-17T14:06:19Z

/test pull-kubernetes-unit

dims · 2017-11-20T11:25:25Z

@luxas @ixdy Can you please add milestone 1.9 and approve it for the milestone too?

Thanks,
Dims

ixdy · 2017-11-20T20:36:09Z

Building the new hyperkube-base images now. You also need to update build/root/WORKSPACE and cluster/images/hyperkube/Makefile for the new tag.

ixdy · 2017-11-20T20:40:25Z

/kind bug
/priority important-soon

Not sure which sig this is.

ixdy · 2017-11-20T20:59:21Z

build/debian-hyperkube-base/Makefile

@@ -19,7 +19,7 @@

 REGISTRY?=gcr.io/google-containers
 IMAGE?=debian-hyperkube-base
-TAG=0.6
+TAG=0.7


hm, #52744 was supposed to bump this to 0.7, but it seems that got lost in a rebase. please bump this to 0.8 instead.

cc @rphillips

cblecker · 2017-11-28T18:12:54Z

/hold cancel
@dims needs rebase
@ixdy should these be updated too?:

kubernetes/build/root/WORKSPACE

Line 94 in 85f0a1a

tag = "0.7", # ignored, but kept here for documentation

kubernetes/cluster/images/hyperkube/Makefile

Line 24 in 85f0a1a

BASEIMAGE=gcr.io/google-containers/debian-hyperkube-base-$(ARCH):0.7

Looks like 0.6, we ended up with dash as the default shell, with /bin/sh as well as /bin/dash ending up invoking dash. We should not change the contract by faking a link to /bin/bash. Let's install the actual bash package and make sure /bin/sh is linked to /bin/bash as well.

dims · 2017-11-28T21:40:44Z

Done with rebase. I can push up another commit with those 2 locations when @ixdy confirms as i am not sure if those should be updated after we push an image to the registry.

cblecker · 2017-11-28T21:50:55Z

@dims I re-read the whole PR, and it looks like he already did (I missed it too):
#55018 (comment)

Building the new hyperkube-base images now. You also need to update build/root/WORKSPACE and cluster/images/hyperkube/Makefile for the new tag.

cblecker · 2017-11-28T23:21:47Z

/lgtm
/assign @zmerlynn @roberthbailey @mikedanese
for cluster approval

k8s-github-robot · 2017-11-28T23:22:09Z

[MILESTONENOTIFIER] Milestone Pull Request Current

@cblecker @dims @ixdy @luxas @mikedanese @roberthbailey @zmerlynn

Note: This pull request is marked as priority/critical-urgent, and must be updated every 1 day during code freeze.

Example update:

ACK.  In progress
ETA: DD/MM/YYYY
Risks: Complicated fix required

Pull Request Labels

sig/node: Pull Request will be escalated to these SIGs if needed.
priority/critical-urgent: Never automatically move pull request out of a release milestone; continually escalate to contributor and SIG through all available channels.
kind/bug: Fixes a bug discovered during the current release.

Help

mikedanese · 2017-11-28T23:52:05Z

/approve

ixdy

/lgtm cancel

ixdy · 2017-11-29T00:00:44Z

build/root/WORKSPACE

@@ -91,7 +91,7 @@ docker_pull(
    digest = "sha256:1a05a58432254268c31ef5c8d9c21f3d01a40611b14707de6ac2772c0793bd13",


you need to update this, or else it's not going to have any effect on bazel builds.

update to sha256:fc1b461367730660ac5a40c1eb2d1b23221829acf8a892981c12361383b3742b and then this lgtm

ixdy · 2017-11-29T00:03:14Z

I think I never got around to building the new base images since I discovered that we'd forgotten to bump to 0.7 previously. as currently written this will fail since the 0.8 tags don't yet exist.

dims · 2017-11-29T02:54:45Z

/test pull-kubernetes-e2e-gce

… the new tag

ixdy · 2017-11-29T19:22:03Z

/lgtm

k8s-github-robot · 2017-11-29T19:22:07Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cblecker, dims, ixdy, mikedanese

Associated issue: 55012

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

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

~~build/OWNERS~~ [cblecker,ixdy,mikedanese]
~~cluster/OWNERS~~ [mikedanese]

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

dims · 2017-11-29T20:59:22Z

/retest

k8s-github-robot · 2017-11-29T23:26:03Z

Automatic merge from submit-queue (batch tested with PRs 56497, 56500, 55018, 56544, 56425). If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-ci-robot added 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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 2, 2017

k8s-github-robot assigned ixdy and zmerlynn Nov 2, 2017

dims force-pushed the use-real-bash-not-dash branch from a6fcd9f to 2ea0899 Compare November 2, 2017 18:14

ixdy reviewed Nov 2, 2017

View reviewed changes

k8s-ci-robot assigned luxas Nov 16, 2017

ixdy reviewed Nov 16, 2017

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 16, 2017

dims force-pushed the use-real-bash-not-dash branch from 2ea0899 to 0dd77eb Compare November 16, 2017 22:35

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 16, 2017

ixdy added this to the v1.9 milestone Nov 20, 2017

k8s-github-robot added the milestone/incomplete-labels label Nov 20, 2017

k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Nov 20, 2017

ixdy reviewed Nov 20, 2017

View reviewed changes

dims force-pushed the use-real-bash-not-dash branch from 0dd77eb to 137c5b9 Compare November 20, 2017 21:44

enisoc added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Nov 21, 2017

k8s-github-robot added milestone/needs-approval and removed milestone/incomplete-labels labels Nov 21, 2017

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 24, 2017

k8s-github-robot added milestone/needs-attention needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 26, 2017

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 28, 2017

k8s-github-robot removed the milestone/needs-attention label Nov 28, 2017

dims force-pushed the use-real-bash-not-dash branch from 5bd917a to e378a2a Compare November 28, 2017 21:35

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 28, 2017

k8s-ci-robot assigned mikedanese and roberthbailey Nov 28, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 28, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 28, 2017

ixdy reviewed Nov 29, 2017

View reviewed changes

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 29, 2017

dims force-pushed the use-real-bash-not-dash branch from 5d6c62b to f16b00b Compare November 29, 2017 19:15

update build/root/WORKSPACE and cluster/images/hyperkube/Makefile for…

f16b00b

… the new tag

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 29, 2017

k8s-github-robot merged commit 216f6d1 into kubernetes:master Nov 29, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not fake /bin/bash, just use the real bash #55018

Do not fake /bin/bash, just use the real bash #55018

dims commented Nov 2, 2017

ixdy commented Nov 2, 2017

ixdy Nov 2, 2017

dims Nov 2, 2017

tallclair commented Nov 2, 2017 •

edited

dims commented Nov 16, 2017

ixdy Nov 16, 2017

dims Nov 16, 2017

dims commented Nov 16, 2017

dims commented Nov 16, 2017

dims commented Nov 17, 2017

dims commented Nov 20, 2017

ixdy commented Nov 20, 2017

ixdy commented Nov 20, 2017

ixdy Nov 20, 2017

dims Nov 20, 2017

cblecker commented Nov 28, 2017

dims commented Nov 28, 2017

cblecker commented Nov 28, 2017

cblecker commented Nov 28, 2017

k8s-github-robot commented Nov 28, 2017

mikedanese commented Nov 28, 2017

ixdy left a comment

ixdy Nov 29, 2017

ixdy Nov 29, 2017

dims Nov 29, 2017

ixdy commented Nov 29, 2017

dims commented Nov 29, 2017

ixdy commented Nov 29, 2017

k8s-github-robot commented Nov 29, 2017

dims commented Nov 29, 2017

k8s-github-robot commented Nov 29, 2017

		@@ -91,7 +91,7 @@ docker_pull(
		digest = "sha256:1a05a58432254268c31ef5c8d9c21f3d01a40611b14707de6ac2772c0793bd13",

Do not fake /bin/bash, just use the real bash #55018

Do not fake /bin/bash, just use the real bash #55018

Conversation

dims commented Nov 2, 2017

ixdy commented Nov 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tallclair commented Nov 2, 2017 • edited

dims commented Nov 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dims commented Nov 16, 2017

dims commented Nov 16, 2017

dims commented Nov 17, 2017

dims commented Nov 20, 2017

ixdy commented Nov 20, 2017

ixdy commented Nov 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cblecker commented Nov 28, 2017

dims commented Nov 28, 2017

cblecker commented Nov 28, 2017

cblecker commented Nov 28, 2017

k8s-github-robot commented Nov 28, 2017

mikedanese commented Nov 28, 2017

ixdy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ixdy commented Nov 29, 2017

dims commented Nov 29, 2017

ixdy commented Nov 29, 2017

k8s-github-robot commented Nov 29, 2017

dims commented Nov 29, 2017

k8s-github-robot commented Nov 29, 2017

tallclair commented Nov 2, 2017 •

edited