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

hyperkube: Build debian-hyperkube-base v1.1.0 image #92354

Merged

Conversation

justaugustus
Copy link
Member

What type of PR is this?

/kind regression
/area dependency
/priority critical-urgent

What this PR does / why we need it:

  • Use debian-iptables v12.1.0 as base image
    Should ensure kube-proxy behaves the same in hyperkube as it would for
    the standard kube-proxy image.
  • Removes the following packages (which are already present in the
    debian-iptables image):
    • conntrack
    • ebtables
    • iptables
    • ipset
    • kmod
    • netbase

Signed-off-by: Stephen Augustus saugustus@vmware.com

Which issue(s) this PR fixes:

Attempt to fix #92275, #92272, #92250.
Should supersede #92281.

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


- Use debian-iptables v12.1.0 as base image
  Should ensure kube-proxy behaves the same in hyperkube as it would for
  the standard kube-proxy image.
- Removes the following packages (which are already present in the
  debian-iptables image):
  - conntrack
  - ebtables
  - iptables
  - ipset
  - kmod
  - netbase

Signed-off-by: Stephen Augustus <saugustus@vmware.com>
@k8s-ci-robot k8s-ci-robot added the do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. label Jun 21, 2020
@k8s-ci-robot k8s-ci-robot added this to the v1.18 milestone Jun 21, 2020
@k8s-ci-robot k8s-ci-robot added kind/regression Categorizes issue or PR as related to a regression from a prior release. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. area/dependency Issues or PRs related to dependency changes do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. sig/release Categorizes an issue or PR as relevant to SIG Release. labels Jun 21, 2020
@k8s-ci-robot k8s-ci-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Jun 21, 2020
@justaugustus
Copy link
Member Author

/assign @BenTheElder @danwinship @cblecker
/area release-eng
cc: @kubernetes/release-engineering

cc-ing reporters/commenters:
cc: @ialidzhikov @vpnachev @juliantaylor @majst01 @waynr @superseb @dmitry-irtegov @aojea

@k8s-ci-robot k8s-ci-robot added the area/release-eng Issues or PRs related to the Release Engineering subproject label Jun 21, 2020
@justaugustus
Copy link
Member Author

/release-note-none

@aojea
Copy link
Member

aojea commented Jun 21, 2020

/lgtm
for the network changes
/hold
for the image dependencies changes, is not something I'm familiar enough to review

@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 Jun 21, 2020
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 21, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder, justaugustus

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-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 24, 2020
@feiskyer feiskyer added cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. and removed do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. labels Jun 24, 2020
@BenTheElder
Copy link
Member

BenTheElder commented Jun 24, 2020 via email

@justaugustus
Copy link
Member Author

/test pull-kubernetes-integration
/test pull-kubernetes-e2e-kind

@justaugustus
Copy link
Member Author

/test pull-kubernetes-integration

@justaugustus
Copy link
Member Author

/test pull-kubernetes-e2e-gce-100-performance

@justaugustus
Copy link
Member Author

/test pull-kubernetes-verify

@dmitry-irtegov
Copy link

I think there is deeper problem with current iptables selection logic.

There are manifests that still are hardcoded to use iptables-legacy. At least, dns-node-cache and flannel.

If one of these manifests is used on the cluster, it creates enough legacy rules to force kube-proxy to legacy mode.

On normal startup it is impossible, but think about a pod restart....

@BenTheElder
Copy link
Member

kube-proxy shouldn't be starting after pods like dns-node-cache and flannel on normal startup, and on pod restart it shouldn't be a problem if the other network tools in use also correctly implement something like this If they use iptables.

If they don't implement this, those tools already left the system broke if the host used iptables / nft not matching them, and kube proxy is matching the host.

@BenTheElder
Copy link
Member

/retest

@justaugustus
Copy link
Member Author

/test pull-kubernetes-e2e-gce-100-performance

@BenTheElder
Copy link
Member

On restart kube-proxy will also have lots of old kube-proxy rules.

@justaugustus
Copy link
Member Author

/test pull-kubernetes-e2e-kind

1 similar comment
@justaugustus
Copy link
Member Author

/test pull-kubernetes-e2e-kind

@BenTheElder
Copy link
Member

BenTheElder commented Jun 24, 2020 via email

@aojea
Copy link
Member

aojea commented Jun 24, 2020

/test pull-kubernetes-e2e-kind

@k8s-ci-robot k8s-ci-robot merged commit 934b415 into kubernetes:release-1.18 Jun 24, 2020
@dmitry-irtegov
Copy link

dmitry-irtegov commented Jun 24, 2020

... if the other network tools in use also correctly implement something like this If they use iptables.

This makes sence. But you probably should mention this in release notes and/or docs. So maintainers of these tools will know what they should do and why.

Also, the recommendation to unload/blacklist ip_tables kernel module might help. So people will know what breaks and why.

@BenTheElder
Copy link
Member

??


1/4992 Tests Failed. | expand_less
-- | --
[sig-node] RuntimeClass should reject a Pod requesting a RuntimeClass with an unconfigured handler expand_less7stest/e2e/common/runtimeclass.go:47 Jun 24 07:08:27.187: Error creating Pod Unexpected error:     <*errors.StatusError \| 0xc001d6e460>: {         ErrStatus: {             TypeMeta: {Kind: "", APIVersion: ""},             ListMeta: {                 SelfLink: "",                 ResourceVersion: "",                 Continue: "",                 RemainingItemCount: nil,             },             Status: "Failure",             Message: "pods \"test-runtimeclass-runtimeclass-1410-unconfigured-handler-\" is forbidden: pod rejected: RuntimeClass \"runtimeclass-1410-unconfigured-handler\" not found",             Reason: "Forbidden",             Details: {                 Name: "test-runtimeclass-runtimeclass-1410-unconfigured-handler-",                 Group: "",                 Kind: "pods",                 UID: "",                 Causes: nil,                 RetryAfterSeconds: 0,             },             Code: 403,         },     }     pods "test-runtimeclass-runtimeclass-1410-unconfigured-handler-" is forbidden: pod rejected: RuntimeClass "runtimeclass-1410-unconfigured-handler" not found occurred test/e2e/framework/pods.go:83 | [sig-node] RuntimeClass should reject a Pod requesting a RuntimeClass with an unconfigured handler expand_less | 7s | test/e2e/common/runtimeclass.go:47 Jun 24 07:08:27.187: Error creating Pod Unexpected error:     <*errors.StatusError \| 0xc001d6e460>: {         ErrStatus: {             TypeMeta: {Kind: "", APIVersion: ""},             ListMeta: {                 SelfLink: "",                 ResourceVersion: "",                 Continue: "",                 RemainingItemCount: nil,             },             Status: "Failure",             Message: "pods \"test-runtimeclass-runtimeclass-1410-unconfigured-handler-\" is forbidden: pod rejected: RuntimeClass \"runtimeclass-1410-unconfigured-handler\" not found",             Reason: "Forbidden",             Details: {                 Name: "test-runtimeclass-runtimeclass-1410-unconfigured-handler-",                 Group: "",                 Kind: "pods",                 UID: "",                 Causes: nil,                 RetryAfterSeconds: 0,             },             Code: 403,         },     }     pods "test-runtimeclass-runtimeclass-1410-unconfigured-handler-" is forbidden: pod rejected: RuntimeClass "runtimeclass-1410-unconfigured-handler" not found occurred test/e2e/framework/pods.go:83
[sig-node] RuntimeClass should reject a Pod requesting a RuntimeClass with an unconfigured handler expand_less | 7s
test/e2e/common/runtimeclass.go:47 Jun 24 07:08:27.187: Error creating Pod Unexpected error:     <*errors.StatusError \| 0xc001d6e460>: {         ErrStatus: {             TypeMeta: {Kind: "", APIVersion: ""},             ListMeta: {                 SelfLink: "",                 ResourceVersion: "",                 Continue: "",                 RemainingItemCount: nil,             },             Status: "Failure",             Message: "pods \"test-runtimeclass-runtimeclass-1410-unconfigured-handler-\" is forbidden: pod rejected: RuntimeClass \"runtimeclass-1410-unconfigured-handler\" not found",             Reason: "Forbidden",             Details: {                 Name: "test-runtimeclass-runtimeclass-1410-unconfigured-handler-",                 Group: "",                 Kind: "pods",                 UID: "",                 Causes: nil,                 RetryAfterSeconds: 0,             },             Code: 403,         },     }     pods "test-runtimeclass-runtimeclass-1410-unconfigured-handler-" is forbidden: pod rejected: RuntimeClass "runtimeclass-1410-unconfigured-handler" not found occurred test/e2e/framework/pods.go:83


@BenTheElder
Copy link
Member

agree re: relnotes @justaugustus

@justaugustus
Copy link
Member Author

??

Where are you seeing that failure?

agree re: relnotes @justaugustus

This PR only builds the new image, which is why it doesn't include a release note.
Follow up PRs will actually utilize the new image (user-facing change) and those will include release notes.

@BenTheElder
Copy link
Member

ack

re the failure: the last kind flake, it's not related to this PR, but it is very ??

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. area/dependency Issues or PRs related to dependency changes area/release-eng Issues or PRs related to the Release Engineering subproject cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/regression Categorizes issue or PR as related to a regression from a prior release. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note-none Denotes a PR that doesn't merit a release note. sig/release Categorizes an issue or PR as relevant to SIG Release. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet