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

v1.8 backports 2020-10-02 #13384

Merged
merged 14 commits into from
Oct 6, 2020
Merged

v1.8 backports 2020-10-02 #13384

merged 14 commits into from
Oct 6, 2020

Conversation

errordeveloper
Copy link
Contributor

@errordeveloper errordeveloper commented Oct 2, 2020

Once this PR is merged, you can update the PR labels via:

$ for pr in 13297 13267 13318 13355 13346 13146 13342 13357 13325 13369 13377; do contrib/backporting/set-labels.py $pr done 1.8; done

@errordeveloper errordeveloper requested a review from a team as a code owner October 2, 2020 10:51
@maintainer-s-little-helper maintainer-s-little-helper bot added backport/1.8 kind/backports This PR provides functionality previously merged into master. labels Oct 2, 2020
@errordeveloper errordeveloper changed the title v1.8 backport 2020 10 02 v1.8 backports 2020-10-02 Oct 2, 2020
kkourt and others added 2 commits October 2, 2020 11:58
[ upstream commit 758539b ]

Initial guide with results and some basic tuning options for users.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
[ upstream commit 5b68613 ]

Added configuration options nodeSelector (deployments) and tolerations (daemonsets and deployments)
for all the existing charts:
  - agent
  - hubble-relay
  - hubble-ui
  - managed-etcd
  - nodeinit
  - operator
  - preflight

On the preflight one, I also simplified the tolerations with a single 'operator: Exists'. Other than
that, the behaviour with default values should remain identical.

Initially, my use case was to be able to avoid having the hubble-relay pods running on tainted nodes.
I went forward with updating all the charts as I felt this could probably be useful for others.

Signed-off-by: Maxime VISONNEAU <maxime.visonneau@gmail.com>
Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
@errordeveloper
Copy link
Contributor Author

test-me-please

Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

There's still one documentation error:

Please fix the following documentation warnings:
/src/Documentation/operations/system_requirements.rst:170: WARNING: undefined label: bandwidth-manager (if the link has no caption the label must precede a section header)

Should be easy to fix with git rebase --exec "make render-docs" master.

[ upstream commit deffa27 ]

Because network namespace cookies are only available in v5.7+, on older
kernels, all pods on a given node will be serviced by the same backend
for a given service, for east-west traffic.

Fixes: 864f2f9 ("docs: Update list of optional kernel requirements")
Reported-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
@errordeveloper
Copy link
Contributor Author

@pchaigno I just deleted the line, sounds like "bandwidth manager" is a 1.9 feature... 🤔

@pchaigno
Copy link
Member

pchaigno commented Oct 2, 2020

@pchaigno I just deleted the line, sounds like "bandwidth manager" is a 1.9 feature... thinking

@errordeveloper Yes, that's correct.

@errordeveloper
Copy link
Contributor Author

@pchaigno for the rest, are you changes looking OK? I think most of them have applied cleanly.

Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

My documentation changes look good. I checked the result by running make render-docs rather than checking the code changes since there are a lot of code changes.

Copy link
Contributor

@kkourt kkourt left a comment

Choose a reason for hiding this comment

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

LGTM for my patches.

kkourt and others added 11 commits October 2, 2020 14:02
[ upstream commit 2371071 ]

- Add multi-stream results
- Add lower/higher is better labels
- use (lat vs batch) and (tput vs batch) plots for TCP_RR
- improve text

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
[ upstream commit 3179a47 ]

It is reasonable to support encapsulation with enable-endpoint-routes.
The existing code derived a new datapath mode when
enable-endpoint-routes was enabled, which automatically disabled
encapsulation.

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
[ upstream commit 1ed8c87 ]

Signed-off-by: Jed Salazar jed@isovalent.com
Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
[ upstream commit 062c942 ]

Signed-off-by: Jed Salazar jed@isovalent.com
Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
[ upstream commit 57d3473 ]

Rather then hardcoding the /sys/fs/bpf value in bugtool, use the
`mountinfo` package (which exposes the information in /proc/self/mounts)
to determine the correct mountpoint for the BPF filesystem.

Fixes: #13218

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
[ upstream commit b71cf0d ]

Due to an extra `v` in the branch name, this script would fail with:

  $ ~/git/cilium/contrib/release/start-release.sh v1.6.12 128
  fatal: 'origin/vv1.6' is not a commit and a branch 'pr/prepare-v1.6.12' cannot be created from it

  Signal ERR caught!

  Traceback (line function script):
  62 main /home/joe/git/cilium/contrib/release/start-release.sh

Fix it.

While we're at it, update the instructions at the end for next
steps, since there's also now a `submit-backport.sh` script to send the
PR from the CLI.

Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
[ upstream commit 28b4c96 ]

fsnotify Event.Op is a bit mask and testing for strict equality might
not detect the event operation correctly.

This patch make it so we check for fsnotify event operation
consistently as documented at https://github.com/fsnotify/fsnotify.

Signed-off-by: Alexandre Perrin <alex@kaworu.ch>
Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
[ upstream commit 7257d4a ]

Since Cilium sets ownership references on pods, it needs permission
to delete pods via finalizers and for that purpose it also needs
permissions to set the finalizers on pods.
This change is required for OpenShift, however it's based on the GC
admission controller that was introduced in Kubernetes 1.5
(kubernetes/kubernetes#34829).

Also add explicit permissions for finalizers on all CRs, to ensure
that agent and operator can set finalizers on their own resources.

Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
[ upstream commit b769c64 ]

Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
[ upstream commit e4b3689 ]

Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
[ upstream commit 55209b7 ]

Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
@errordeveloper
Copy link
Contributor Author

@rolinh I'm dropping #13324 as discussed, since conflicts are beyond trivial and deserve a separate PR.

@errordeveloper errordeveloper removed the request for review from gandro October 2, 2020 13:04
@errordeveloper
Copy link
Contributor Author

test-me-please

@errordeveloper
Copy link
Contributor Author

test-gke

@errordeveloper
Copy link
Contributor Author

test-k8s-1.19

@pchaigno
Copy link
Member

pchaigno commented Oct 5, 2020

test-backport-1.8

Copy link
Member

@kaworu kaworu left a comment

Choose a reason for hiding this comment

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

LGTM for my patches, thanks!

@pchaigno
Copy link
Member

pchaigno commented Oct 6, 2020

retest-4.19

@errordeveloper
Copy link
Contributor Author

errordeveloper commented Oct 6, 2020

So I can see this in Jenkins:

14:32:58  Successfully tagged cilium/operator-generic:d901575443ee90b7979d1f24477ae80b2cfdfe30
14:32:58  Push like this when ready:
14:32:58  docker push cilium/operator-generic:d901575443ee90b7979d1f24477ae80b2cfdfe30-amd64
14:32:58  make[1]: Leaving directory '/home/jenkins/workspace/Cilium-PR-K8s-GKE/src/github.com/cilium/cilium'
14:32:59  The push refers to repository [147.75.55.179/cilium/cilium]
14:32:59  Get https://147.75.55.179/v2/: x509: cannot validate certificate for 147.75.55.179 because it doesn't contain any IP SANs
Post stage
[Pipeline] script
[Pipeline] {
[Pipeline] }
[Pipeline] // script
[Pipeline] }
[Pipeline] // stage
[Pipeline] }
14:32:59  Failed in branch Make Cilium images
[Pipeline] // parallel
[Pipeline] }
[Pipeline] // stage
[Pipeline] stage
[Pipeline] { (BDD-Test-PR)
Stage "BDD-Test-PR" skipped due to earlier failure(s)
[Pipeline] }
[Pipeline] // stage
[Pipeline] stage
[Pipeline] { (Declarative: Post Actions)
[Pipeline] sh
14:33:00  + cd /home/jenkins/workspace/Cilium-PR-K8s-GKE/src/github.com/cilium/cilium/test
14:33:00  + ./print-node-ip.sh
14:33:00  + ./clean-local-registry-tag.sh 147.75.55.179 d901575443ee90b7979d1f24477ae80b2cfdfe30
14:33:00  1.31.1: Pulling from library/busybox
14:33:00  Digest: sha256:95cf004f559831017cdf4628aaf1bb30133677be8702a8c5f2994629f637a209
14:33:00  Status: Image is up to date for busybox:1.31.1
14:33:00  docker.io/library/busybox:1.31.1
14:33:01  The push refers to repository [147.75.55.179/cilium/cilium]
14:33:01  Get https://147.75.55.179/v2/: x509: cannot validate certificate for 147.75.55.179 because it doesn't contain any IP SANs
14:33:01  + true
[Pipeline] sh
14:33:02  + cd /home/jenkins/workspace/Cilium-PR-K8s-GKE/src/github.com/cilium/cilium/test/gke
14:33:02  + ./release-cluster.sh
14:33:02  Deleting cluster cilium-ci-18...
14:35:16  .....................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................done.
14:35:16  Deleted [https://container.googleapis.com/v1/projects/cilium-ci/zones/us-west1-b/clusters/cilium-ci-18].

It looks like an issue with local registry.

@nebril can we ignore this for this 1.8 backport?

@nebril
Copy link
Member

nebril commented Oct 6, 2020

@errordeveloper let's retest for both failed builds, just to be on the safe side (K8s-1.19-kernel-4.9 failed on provisioning).

@nebril
Copy link
Member

nebril commented Oct 6, 2020

test-gke

@nebril
Copy link
Member

nebril commented Oct 6, 2020

test-4.9

@pchaigno
Copy link
Member

pchaigno commented Oct 6, 2020

The builds that failed due to provisioning issues are not required for backport PRs (is it even expected that they fail?). All other builds are passing, so marking as ready-to-merge.

@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 6, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 6, 2020
@gandro gandro merged commit c0e9c5f into v1.8 Oct 6, 2020
@gandro gandro deleted the pr/v1.8-backport-2020-10-02 branch October 6, 2020 10:36
@tgraf
Copy link
Member

tgraf commented Oct 6, 2020

Was too late, reviewed my backported commit. LGTM 👍

@errordeveloper
Copy link
Contributor Author

Labels updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/backports This PR provides functionality previously merged into master.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants