test images: Image Promoter fixes #87188
Conversation
Prior to the Image Centralization part 4 (kubernetes#81170), a PR merged that enables the Image Promoter to run on the k/k test images. The Image Promoter currently only builds the Conformance-related images, but the Image Centralization part 4 centralized some of those images into agnhost, so they need to be removed from the conformance_images list. Additionally, kubernetes#81226 proposes mounttest-user image to be removed, and RunAsUser to be used in tests instead. The image used by the Image Promoter (gcr.io/k8s-testimages/gcb-docker-gcloud:v20190906-745fed4) is based on busybox, and thus, the sed binary is actually busybox. image-util.sh calls kube::util::ensure-gnu-sed several times, which ensures that a GNU sed binary exists (it checks by greping GNU in its --help output). Obviously, it won't match the busybox sed binary. But the sed usage in image-util.sh is fairly simple, and the busybox sed is sufficient. Bumps image versions for: jessie-dnsutils, nonewprivs, resource-consumer, sample-apiserver. These images are included in the conformance_images that are being built by the Image Promoter, so we're bumping them just to make sure we're not breaking anything and cause all the CIs to fall. We're going to bump the image versions used in tests in a subsequent PR. The image version was not bumped for: agnhost, kitten, nautilus, as they were already bumped by the Image Centralization part 4 PR.
k8s-ci-robot
added
release-note-none
|
/test pull-kubernetes-e2e-gce |
claudiubelu
force-pushed
the
test-images/centralization-cleanup
branch
from
fe6f603
to
0753814
Compare
claudiubelu
changed the title
claudiubelu
force-pushed
the
test-images/centralization-cleanup
branch
from
0753814
to
f81c899
Compare
k8s-ci-robot
added
size/M
hack/lib/util.sh
Outdated
| if LANG=C sed --help 2>&1 | grep -q GNU; then | ||
| # NOTE: the echo below is a workaround to ensure sed is executed before the grep. | ||
| # see: https://github.com/kubernetes/kubernetes/issues/87251 | ||
| if echo "$(LANG=C sed --help 2>&1)" | grep -q "GNU\|BusyBox"; then |
There was a problem hiding this comment.
I strongly doubt that the shell runs the piped commands randomly run out-of-order. The stdout logs that we've seen is probably a result of the buffered output being re-combined incorrectly.
There was a problem hiding this comment.
Indeed, it sounds incredible, but it is also the only explanation as to why that function would randonly fail.
There was a problem hiding this comment.
Are we sure that this function fails randomly on the same system, and not that the function is failing (seemingly) randomly on different systems that have (or don't have) GNU sed?
There was a problem hiding this comment.
This is basically what is being executed: https://paste.ubuntu.com/p/2jHTmfCvpP/
In my ubuntu 16.04 (GNU bash, version 5.0.0(1)-release (x86_64-pc-linux-gnu) and my 18.04 (GNU bash, version 4.4.20(1)-release (x86_64-pc-linux-gnu) OSes, it passed for all 200 runs.
While for the gcr.io/k8s-testimages/gcb-docker-gcloud:v20190906-745fed4 image ("Alpine Linux v3.10") (GNU bash, version 5.0.0(1)-release (x86_64-alpine-linux-musl), the number of failures varies between 19 and 98.
PS: With this change, the failure rate drops to virtually 0 (5 / 3200):
if echo "$(LANG=C sed --help 2>&1)" | grep -q "GNU\|BusyBox"; then
With this change ,the failure rate is 0 ( 0 / 4000):
sed_help="$(LANG=C sed --help 2>&1)"
if echo "${sed_help}" | grep -q "GNU\|BusyBox"; then
f81c899
to
4c51eb9
Compare
|
/retest |
1 similar comment
|
/retest |
|
/test pull-kubernetes-e2e-kind |
|
/approve |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: claudiubelu, spiffxp 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 |
k8s-ci-robot
added
the
approved
|
/retest |
The image used by the Image Promoter (gcr.io/k8s-testimages/gcb-docker-gcloud:v20190906-745fed4) is based on busybox, and thus, the sed binary is actually busybox. image-util.sh calls kube::util::ensure-gnu-sed several times, which ensures that a GNU sed binary exists (it checks by greping GNU in its --help output). Obviously, it won't match the busybox sed binary. But the sed usage in image-util.sh is fairly simple, and the busybox sed is sufficient. This was previously fixed in: kubernetes#87188, but it was reverted by kubernetes#87653 as it was failing on Mac (sed does not exist). This commit fixes that issue as well.
What type of PR is this?
/kind cleanup
/sig testing
/area conformance
What this PR does / why we need it:
Prior to the Image Centralization part 4 (#81170), a PR merged that enables the Image Promoter to run on the k/k test images.
The Image Promoter currently only builds the Conformance-related images, but the Image Centralization part 4 centralized some of those images into
agnhost, so they need to be removed from theconformance_imageslist.Additionally, #81226 proposes
mounttest-userimage to be removed, andRunAsUserto be used in tests instead. This was proposed here: #76342The image used by the Image Promoter (
gcr.io/k8s-testimages/gcb-docker-gcloud:v20190906-745fed4) is based on busybox, and thus, the sed binary is actually busybox.image-util.shcallskube::util::ensure-gnu-sedseveral times, which ensures that a GNU sed binary exists (it checks by greping GNU in its --help output). Obviously, it won't match the busybox sed binary. But the sed usage inimage-util.shis fairly simple, and the busybox sed is sufficient.Bumps image versions for:
jessie-dnsutils,nonewprivs,resource-consumer,sample-apiserver. These images are included in theconformance_imagesthat are being built by the Image Promoter, so we're bumping them just to make sure we're not breaking anything and cause all the CIs to fall. We're going to bump the image versions used in tests in a subsequent PR. The image version was not bumped for:agnhost,kitten,nautilus, as they were already bumped by the Image Centralization part 4 PR.Which issue(s) this PR fixes:
Related: #76342
Related: #81226
Related: #81170
Related: #87251
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: