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

Add ipset binary for IPVS, context: Fixes #57321 #57648

Merged

Conversation

Fsero
Copy link

@Fsero Fsero commented Dec 27, 2017

What this PR does / why we need it: Add ipset binary in debian-hyperkube-base which fixes issue 57321

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 #57321

Special notes for your reviewer:

Release note:

Add ipset binary for IPVS to hyperkube docker image

@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 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. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 27, 2017
@Fsero Fsero changed the title Add ipset binary for IPVS, context: https://github.com/kubernetes/kub… Add ipset binary for IPVS, context: Fixes #57321 Dec 27, 2017
@Fsero Fsero changed the base branch from master to release-1.9 December 27, 2017 10:48
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 27, 2017
@Fsero Fsero changed the base branch from release-1.9 to master December 27, 2017 10:49
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 27, 2017
@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
  • If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]

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 k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. release-note-none Denotes a PR that doesn't merit a release note. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. 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 Dec 27, 2017
@dims
Copy link
Member

dims commented Dec 27, 2017

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 27, 2017
@cblecker
Copy link
Member

Looks reasonable to me.

@Fsero Would you mind bumping the tag too?

@Fsero Fsero force-pushed the bugfix/add_ipset_binary_for_ipvs branch from 76daecc to bb91e08 Compare December 27, 2017 19:27
@Fsero
Copy link
Author

Fsero commented Dec 27, 2017

@cblecker done! thanks for the review

@Fsero
Copy link
Author

Fsero commented Dec 27, 2017

/assign @cblecker

@cblecker
Copy link
Member

@Fsero Sorry, a couple other places that need updates too:

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

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

@Fsero
Copy link
Author

Fsero commented Dec 27, 2017

@cblecker sure, i'll change it and review it more. Thanks!

@Fsero Fsero force-pushed the bugfix/add_ipset_binary_for_ipvs branch from bb91e08 to bc6b0de Compare December 27, 2017 21:06
@Fsero
Copy link
Author

Fsero commented Dec 27, 2017

@cblecker changed and reviewed, it looks like this is it

@Fsero Fsero force-pushed the bugfix/add_ipset_binary_for_ipvs branch from bc6b0de to 6fd8903 Compare February 11, 2018 12:39
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 11, 2018
@cblecker
Copy link
Member

@ixdy What else is needed here?

@ixdy
Copy link
Member

ixdy commented Feb 12, 2018

@cblecker I was waiting for the WORKSPACE update.

we probably need to push a new image and update the hash again since #56937 merged ahead of this.

@ixdy
Copy link
Member

ixdy commented Feb 13, 2018

@Fsero please update build/root/WORKSPACE to bump the debian-hyperkube-base-amd64 digest there. The new 0.9 tag has digest sha256:d83594ecd85345144584523e7fa5388467edf5d2dfa30d0a1bcbf184cddf4a7b.

@ixdy
Copy link
Member

ixdy commented Feb 13, 2018

the patch should look something like:

diff --git a/build/root/WORKSPACE b/build/root/WORKSPACE
index 88f35e1eaf..adf0eed006 100644
--- a/build/root/WORKSPACE
+++ b/build/root/WORKSPACE
@@ -67,10 +67,10 @@ docker_pull(
 
 docker_pull(
     name = "debian-hyperkube-base-amd64",
-    digest = "sha256:fc1b461367730660ac5a40c1eb2d1b23221829acf8a892981c12361383b3742b",
+    digest = "sha256:d83594ecd85345144584523e7fa5388467edf5d2dfa30d0a1bcbf184cddf4a7b",
     registry = "k8s.gcr.io",
     repository = "debian-hyperkube-base-amd64",
-    tag = "0.8",  # ignored, but kept here for documentation
+    tag = "0.9",  # ignored, but kept here for documentation
 )
 
 docker_pull(

k8s-github-robot pushed a commit that referenced this pull request Feb 13, 2018
Automatic merge from submit-queue (batch tested with PRs 59298, 59773, 59772). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

bazel: update digests for debian-iptables-amd64 and busybox

**What this PR does / why we need it**: I've pushed updated (rebased) versions of the `debian-base-ARCH:0.3` and `debian-iptables-ARCH:v10` images. Since bazel uses the sha256 digest instead of the tag, we need to update those accordingly.

I also bumped the busybox digest, which hasn't been updated since last summer. This is updating it from v1.26.2 to v1.28.0. Note that the non-bazel build process uses `busybox:latest`, and so has already been using busybox v1.28.0.

**Special notes for your reviewer**:
We will update the hyperkube-base image in #57648.

**Release note**:

```release-note
NONE
```

/assign @tallclair 
/cc @rphillips @rvkubiak
Copy link

@ivanilves ivanilves left a comment

Choose a reason for hiding this comment

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

❤️

@ivanilves
Copy link

Why can't we just merge this awesome PR? 😇

@christianhuening
Copy link

christianhuening commented Feb 21, 2018

Yeah please merge that. We need LGTM on this one! @jbeda ?

@cblecker
Copy link
Member

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 21, 2018
@Fsero
Copy link
Author

Fsero commented Feb 21, 2018

@ixdy sorry it went under my radar, i've just updated it

@cblecker
Copy link
Member

/hold cancel
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Feb 21, 2018
@ixdy
Copy link
Member

ixdy commented Feb 21, 2018

perfect, thanks!
/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cblecker, Fsero, gyliu513, ixdy

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

@ixdy
Copy link
Member

ixdy commented Mar 28, 2018

I think in one of the rebases this missed an update to cluster/images/hyperkube/Makefile, so none of the official releases (e.g. 1.10.0) are using this yet. We can cherrypick a fix to 1.10 if we want to fix this for 1.10.1.

@rphillips
Copy link
Member

Got a cherry pick up for 1.10 #61861

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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/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.

IPset missing from gcr.io/google_containers/hyperkube-amd64