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.11 backports 2022-02-07 #18726

Merged
merged 31 commits into from
Feb 8, 2022
Merged

Conversation

joamaki
Copy link
Contributor

@joamaki joamaki commented Feb 7, 2022

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

$ for pr in 18583 18563 18277 18478 18671 18643 18682 18676 18655 18679 18697 18693 18684 18707 18486; do contrib/backporting/set-labels.py $pr done 1.11; done

or with

$ make add-label branch=v1.11 issues=18583,18563,18277,18478,18671,18643,18682,18676,18655,18679,18697,18693,18684,18707

vannyle and others added 30 commits February 7, 2022 17:29
[ upstream commit e83c818 ]

Add the following text to documentation:
Hands-on tutorial in a live environment to quickly get started with Cilium.

Signed-off-by: Van Le <vannnyle@gmail.com>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 1287c63 ]

Signed-off-by: Aditi Ghag <aditi@cilium.io>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit ab88f85 ]

The configuration option is moved from extra args -
https://artifacthub.io/packages/helm/uswitch/kiam/5.9.0.

Signed-off-by: Aditi Ghag <aditi@cilium.io>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit fcbeb64 ]

Commits da35c88 and 0ab4fa1 introduced a regression for
local-redirect use cases like kiam, whereby host networked pod
updates were skipped. As a result, node-local redirection for
cases where LRPs select host networked pods as backends broke.

Tested the fix on an EKS configured with kiam setup.

Fixes: da35c88 ("k8s/watchers: don't silently ignore (*K8sWatcher).updatePodHostData error")
Fixes: 0ab4fa1 (pkg/k8s: ignore certain ipcache errors)
Fixes: cilium#16920

Signed-off-by: Aditi Ghag <aditi@cilium.io>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 66967dd ]

Test kiam like use cases (LRP select host networked pods) with an address
matcher LRP -
https://docs.cilium.io/en/latest/gettingstarted/local-redirect-policy/#addressmatcher.

Signed-off-by: Aditi Ghag <aditi@cilium.io>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 9020fda ]

Signed-off-by: Aditi Ghag <aditi@cilium.io>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
…server

[ upstream commit d76a06b ]

Commandline arguments accessed through the option.Config object
(eg. --identity-allocation-mode, --kvstore-opt) were not processed properly.
Function option.Config.Populate() was called too soon (before calling of
rootCmd.Execute(), but os.Args are processed just by rootCmd.Execute()).

Also there was missing debug log-level setting so debug messages did not work at all.

Signed-off-by: adam.bocim <adam.bocim@firma.seznam.cz>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 768659f ]

As mentioned in below upstream issue, there is a discrepancy in viper
while reading string map string data type i.e. kv pair format was not
supported, only `{"k":"v"}` format is allowed. This commit is to wrap
GetStringMapString implementation to handle such case.

Also, during the bootstrap, if there is any flag having invalid value,
fatal log will be printed for early detection and awareness.

Relates spf13/viper#911
Fixes cilium#18328

Signed-off-by: Tam Mach <tam.mach@isovalent.com>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit ee7b402 ]

The Set() function for this struct is expecting k=v format,
however, the String() implementation is not returning same format,
which causes some issue while parsing for few fields such as
kvstore-opt, and api-rate-limit.

Relates to cb14db88d7

Signed-off-by: Tam Mach <tam.mach@isovalent.com>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 363cee2 ]

We already suggest to disable k3s network policy enforcement in the
quick start guide, see commit 1178b04 ("docs: disable k3s network
policy enforcement") and commit 7bd301b ("docs(k3s): add back the
flag to disable network policies").

Suggest doing so in the k3s-specific advanced installation guide as
well.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit ab891ae ]

Explicity set `KUBECONFIG` when using `cilium install`, otherwise
installation might target the wrong cluster.

Follow commit 606b5fe ("docs: KUBECONFIG for cilium-cli with k3s")
which already changed this in the quick start guide.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit cdda826 ]

When IPSec is enabled, it is helpful to trace ESP packets as encrypted.
Fix the current program to do that. In non-IPSec mode, it won't happen
because we are not interested in the encrypted packets. Please see the
comment as well for more details.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 2cfff53 ]

When IPSec is enabled, it is helpful to trace ESP packets as encrypted.
Fix the current program to do that. In non-IPSec mode, it won't happen
because we are not interested in the encrypted packets. Please see the
comment as well for more details.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 0d2672b ]

In do_netdev_encrypt for native routing mode, there are some drop
notifications available. However, they are missing source identity
currently. With the change in 144e9e0, we can pass inherited source
identity from mark to do_netdev_encrypt, so use that.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 800f1ed ]

The review-requested:@me filter lists pull requests that expect a review
from you or from any team you belong to. To allow everyone to see only
pull requests assigned specifically to them, we had to teach MLH to
assign individual reviewers to the pull request and then use
assignee:@me instead of review-requested:@me.

Now that GitHub has a user-review-requested filter, we don't need to
rely on the assignee filter anymore for reviews. To view pull requests
assigned specifically to you (and not just to one of the teams you
belong to), you can use user-review-request:@me.

There is one small difference in behavior between the two filters.
assignee:@me would display pull requests you already reviewed but gave a
Request changes or Comment review. Conversely, user-review-requested:@me
will only display pull requests you haven't reviewed yet or for which
the author re-requested a review from you.

Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 36ff39b ]

The test doesn't have anything to do with the integration tests, and we
had enough confidence that the test script is correct.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 678aff9 ]

Since we have bumped the CI net-next job's k8s version to 1.23, on that
job we have the full DualStack support. This means that instead of
manually provisioning IPv6 services, we can rely on k8s to do so.

With the removal we loose bpf_lxc IPv6 coverage for the LB parts, as on
the net-next bpf_sock is doing the E/W xlations. The coverage will be
retrieved in the subsequent commit.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit bc095ad ]

Previous commit dropped the manual DualStack test cases which we run on
the 4.9 CI job. The job runs with KPR=disabled, to it was testing IPv6
ClusterIP for the per-packet LB (bpf_lxc).

The ClusterIP IPv6 in bpf_lxc should be covered by the smoke-v6 suite
run in the GH action. However, it's unclear whether the suite is running
with KPR=disabled or enabled (it's up to cilium-cli). So to make sure
that we don't accidentally loose the coverage, add the IPv6 ClusterIP
tests to the test case which disables the socket-lb for pod netns.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 2cbb592 ]

This commit removes the "Checks ClusterIP connectivity on the same node"
test case which was wrongly assuming that it queried the service
endpoint running on the same node.

Instead of fixing the test case, the commit removes it, as testing on
the same node would not give any new coverage wrt existing "across the
nodes" test case.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 70d3f99 ]

It has nothing to do with the k8s services.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit d7f1518 ]

The test suite was used to test the L2 <-> L3 device redirection in the
NodePort BPF. However, the test case turned to be flake.

Anyway, we are planning to add the WireGuard support for KPR which would
make the L2 <-> L3 testing much easier. So for now, just remove the
non-used test case.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 8f388fe ]

Instead of having the dedicated CI test case to test service-proxy-name
functionality, create a unit test with the fake client-go instance to
test whether the label filtering is working properly.

This is part of ongoing work to reduce the k8sT/Services.go size and
flakiness.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit b4c9ba1 ]

Previously, commit 606b5fe ("docs: KUBECONFIG for cilium-cli with
k3s") and commit 606b5fe ("docs: KUBECONFIG for cilium-cli with
k3s") changed `cilium install` commands to set KUBECONFIG explicitly.

However, successive cilium-cli commands (e.g. `cilium status` or
`cilium connectivity test` in the getting started guide) will need the
corresponding kubeconfig as well. Thus, suggest to `export KUBECONFIG`
at the top of the k3s specific guides.

For cilium/cilium-cli#696

Signed-off-by: Tobias Klauser <tobias@cilium.io>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 422d7fc ]

Cilium treats label patterns as regular expressions. The existing
default labels, e.g. "!k8s.io", used a '.', which matches any character.
This led to the default labels being too permissive in their matching
and consequently labels like "k8sXo" being excluded from the identity,
with consequent security implications.

This commit properly escapes the regular expressions used in the default
labels.

Signed-off-by: Tom Payne <tom@isovalent.com>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit cf39553 ]

Signed-off-by: Tom Payne <tom@isovalent.com>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit d067c58 ]

It's going to be used by the BGP LB suite which is going to be moved to
a separate file.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit d6e0619 ]

The suite is testing mostly the BGP integration in the agent, and the LB
functionality is already tested by k8sT/Services.go. In addition, we
will see more BGP tests with the ongoing BGP work. So it makes sense to
move it to the separate suite.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit d0177cb ]

And make it public. The test/helpers package is a better place for the
module.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 4b49be4 ]

The test cases doesn't have anything to do with the LB, and they only
checking the plumbing (with questionable assertions, but that's another
topic). So, K8sPolicyTest is a better home for it.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 96f4050 ]

This timeout can be too small when the host has to download all boxes
due to not having any of the boxes required for the SHA to be tested.

In particular this is prone to happen on backport PRs, since it's more
likely for the job to be scheduled on a node that primarily run `master`
pipelines up to that point.

Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
@maintainer-s-little-helper maintainer-s-little-helper bot added backport/1.11 This PR represents a backport for Cilium 1.11.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master. labels Feb 7, 2022
@joamaki
Copy link
Contributor Author

joamaki commented Feb 7, 2022

@brb the commits that moved tests around ("test: Move ...") needed some bigger changes, so could you compare them and see if the changes I made were sensible?

Copy link
Member

@nbusseneau nbusseneau left a comment

Choose a reason for hiding this comment

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

My changes LGTM.

@joamaki
Copy link
Contributor Author

joamaki commented Feb 7, 2022

/test-backport-1.11

Job 'Cilium-PR-K8s-1.19-kernel-4.9' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sDatapathConfig Host firewall With native routing

Failure Output

FAIL: Failed to reach 192.168.56.11:80 from testserver-host-92fqx

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-1.19-kernel-4.9 so I can create a new GitHub issue to track it.

Job 'Cilium-PR-K8s-1.17-kernel-4.9' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sPolicyTest Multi-node policy test with L7 policy using connectivity-check to check datapath

Failure Output

FAIL: cannot install connectivity-check

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-1.17-kernel-4.9 so I can create a new GitHub issue to track it.

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 change looks good. Thanks!

Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

Looks good for my commits, thanks 🥇

Copy link
Member

@aditighag aditighag left a comment

Choose a reason for hiding this comment

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

Backport of my PR looks good. Thanks!

@joamaki
Copy link
Contributor Author

joamaki commented Feb 8, 2022

/test-backport-1.11

Job 'Cilium-PR-K8s-GKE' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sDatapathConfig Host firewall Check connectivity with IPv6 disabled

Failure Output

FAIL: Pods are not ready in time: timed out waiting for pods with filter  to be ready: 4m0s timeout expired

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-GKE so I can create a new GitHub issue to track it.

Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

My changes LGTM, thanks.

@joamaki
Copy link
Contributor Author

joamaki commented Feb 8, 2022

/test-runtime

@joamaki
Copy link
Contributor Author

joamaki commented Feb 8, 2022

/test-gke

@joamaki joamaki merged commit def3259 into cilium:v1.11 Feb 8, 2022
@YutaroHayakawa YutaroHayakawa removed their assignment Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.11 This PR represents a backport for Cilium 1.11.x of a PR that was merged to main. 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