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.

Sign up for GitHub

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

Jump to bottom

v1.8 backports 2021-06-10 #16497

Merged
merged 13 commits into from
Jun 16, 2021
Merged

v1.8 backports 2021-06-10 #16497

merged 13 commits into from
Jun 16, 2021

Conversation

gandro
Copy link
Member

@gandro gandro commented Jun 10, 2021 •
edited

Skipped

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

$ for pr in 14594 16268 16271 16176 16137 16055 16118 16016 16432 16408 16186; do contrib/backporting/set-labels.py $pr done 1.8; done

@gandro gandro requested a review from a team as a code owner June 10, 2021 16:48
@gandro gandro added backport/1.8 kind/backports This PR provides functionality previously merged into master. labels Jun 10, 2021
qmonnet
qmonnet reviewed Jun 10, 2021
View reviewed changes
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

#16137 -- docs: document the policy for backporting documentation changes (@qmonnet)
#16408 -- docs: add a "Copy Commands" button for shell-session snippets (@qmonnet)

Those are good, thank you!

@qmonnet qmonnet removed their assignment Jun 10, 2021
@gandro
Copy link
Member Author

gandro commented Jun 10, 2021 •
edited

test-backport-1.8

aanm
aanm approved these changes Jun 10, 2021
View reviewed changes
Copy link
Member

@aanm aanm 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 changes

@gandro
Copy link
Member Author

gandro commented Jun 14, 2021 •
edited

All tests with visibility annotation failed. Could this be caused by 34be426 @jrajahalme ?

Example:

/home/jenkins/workspace/Cilium-PR-K8s-1.15-kernel-4.9/src/github.com/cilium/cilium/test/ginkgo-ext/scopes.go:514
timed out waiting for endpoint to regenerate for visibility annotation
Expected
    <*errors.errorString | 0xc0013247c0>: {
        s: "<ingress/80/tcp/http> is not in the output after timeout: 4m0s timeout expired",
    }
to be nil

brb
brb approved these changes Jun 14, 2021
View reviewed changes
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.

LGTM for my changes, thanks.

christarazi and others added 4 commits June 14, 2021 18:31
@christarazi @gandro
docs: Recommend use of dev VM for backporting
092cef7 
[ upstream commit 7a4184f ]

This will reduce chances of users using their own vagrant VMs which may
come with libraries that are incompatible with our dependencies.

Signed-off-by: Chris Tarazi <chris@isovalent.com>
Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@jibi @gandro
node: fix arpping test
f9bbd6f 
[ upstream commit 5a418a3 ]

In TestArpPingHandling, wait for all goroutines that are inserting the
new neighbors to finish before deleting the node.

Fixes: cilium#16221
Suggested-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@qmonnet @gandro
docs: add a "Copy Commands" button for shell-session snippets
e258657 
[ upstream commit 869e678 ]

Add a "Copy Commands" to some code blocks. This new button attempts to
copy only commands (and not their output) to the clipboard. The
distinction between commands and output relies on the presence of a
prompt symbol, either "$" or "#", at the beginning of the commands. If a
command ends with a trailing backslash, copy the next line as well.

For example, the following snippet:

    .. code-block:: shell-session

        $ ls -l
        foo
        cat
        $ echo 1 \
        2 \
                3\
            4
        $nospace
        # exit

should place the following text into the clipboard:

    ls -l
    echo 1
    2
            3
        4
    exit

The button is added for the following blocks, when they contain several
lines and at least one command is found in the block:

- "code-block", but with language "shell-session" only,
- Literal blocks ("::"),
- Parsed literals.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@gandro
docs: ENIs should not be unmanaged by the OS
f661c10 
[ upstream commit b15cee1 ]

When ENIs are managed by services such as NetworkManager or
systemd-networkd, it can happen that they interfere with Cilium's
configuration. For example, systemd-networkd can remove the ENI IP
assigned by Cilium if the carrier is temporarily down, thus breaking
SNAT.

We previously had a similar section regarding NetworkManager and DHCP in
the EKS installation guide, but the EKS guide has since been replaced by
the Cilium CLI installation guide.

This section here therefore acts as a replacement and states that the
devices need to be unmanaged (e.g. disabling DHCP is not enough for
systemd-networkd).

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@gandro gandro force-pushed the pr/v1.8-backport-2021-06-10 branch from bf0f43c to f661c10 Compare June 14, 2021 16:32
@gandro
Copy link
Member Author

gandro commented Jun 14, 2021

test-backport-1.8

@gandro
Copy link
Member Author

gandro commented Jun 15, 2021 •
edited

Removing 29e6b13 to test if it was the culprit

@aanm I had to drop #16190 as it was breaking all tests which used policy visibility annotations. Please take a look, maybe my conflict resolution was buggy.

See #16497 (comment) for Jenkins output.

@gandro
Copy link
Member Author

gandro commented Jun 15, 2021 •
edited

CI hit various flakes on 1.8:

@gandro gandro marked this pull request as ready for review June 15, 2021 08:38
@gandro
Copy link
Member Author

gandro commented Jun 15, 2021

test-1.12-4.9

@gandro
Copy link
Member Author

gandro commented Jun 15, 2021

test-1.14-4.9

@gandro
Copy link
Member Author

gandro commented Jun 15, 2021

test-1.18-4.9

@gandro
Copy link
Member Author

gandro commented Jun 15, 2021

test-upstream-k8s

@gandro
Copy link
Member Author

gandro commented Jun 15, 2021

test-runtime

@gandro
Copy link
Member Author

gandro commented Jun 15, 2021

Hit the same flakes again:

https://jenkins.cilium.io/job/Cilium-PR-Runtime-4.9/4991/ arping #16075 (i.e. not the one addressed in this backport)
https://jenkins.cilium.io/job/Cilium-PR-K8s-1.14-kernel-4.9/291/ go: RLock /cilium/go.mod: no locks available

@gandro
Copy link
Member Author

gandro commented Jun 15, 2021

test-1.14-4.9

@gandro
Copy link
Member Author

gandro commented Jun 15, 2021

test-runtime

@gandro gandro removed the request for review from bmcustodio June 15, 2021 15:18
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 16, 2021
@aditighag aditighag merged commit ce997f7 into cilium:v1.8 Jun 16, 2021
@aditighag
Copy link
Member

aditighag commented Jun 16, 2021 •
edited

Setting labels for PR 16271... backport-pending/1.8 label not found in PR, exiting

Saw this while updating the labels. @gandro Was there an issue in the script setting backport-pending label on that PR? 🤔

Edit : Looks like the backport-pending label wasn't updated after the commits were re-added - #16497 (comment).

@aditighag aditighag mentioned this pull request Jun 16, 2021
Merged
@jibi jibi mentioned this pull request Jun 17, 2021
Closed
@jibi
Copy link
Member

jibi commented Jun 17, 2021 

* runtime hit #16075 - looks like a different test than the one fixed by [f9bbd6f](https://github.com/cilium/cilium/commit/f9bbd6fc5e68f57498b497eb348a118ef8a49f7b) ? https://jenkins.cilium.io/job/Cilium-PR-Runtime-4.9/4978/ @jibi @brb

Looks like the same test (linuxPrivilegedIPv4OnlyTestSuite.TestArpPingHandling) but it's failing in a different way now:

	 node_linux_test.go:1198:
	     c.Assert(n.HardwareAddr.String(), check.Equals, mac.String())
	 ... obtained string = "02:48:4e:57:94:3c"
	 ... expected string = "7e:06:77:f1:c6:7d"
	
	 level=info msg="Unable to insert neighbor" error="no such device" hardwareAddr="7e:06:77:f1:c6:7d" interface=veth0 ipAddr=9.9.9.250 subsys=node-neigh-debug
	 level=info msg="Unable to insert neighbor" error="no such device" hardwareAddr="7e:06:77:f1:c6:7d" interface=veth0 ipAddr=9.9.9.250 subsys=node-neigh-debug
	 level=info msg="Unable to insert neighbor" error="no such device" hardwareAddr="7e:06:77:f1:c6:7d" interface=veth0 ipAddr=9.9.9.250 subsys=node-neigh-debug
	 level=info msg="Unable to remove neighbor entry" error="no such device" hardwareAddr="7e:06:77:f1:c6:7d" interface=veth0 ipAddr=9.9.9.250 linkIndex=56 subsys=node-neigh-debug
	 level=info msg="Unable to insert neighbor" error="no such device" hardwareAddr="7e:06:77:f1:c6:7d" interface=veth0 ipAddr=9.9.9.250 subsys=node-neigh-debug
	 level=info msg="Unable to insert neighbor" error="no such device" hardwareAddr="7e:06:77:f1:c6:7d" interface=veth0 ipAddr=9.9.9.250 subsys=node-neigh-debug
	 level=info msg="Unable to insert neighbor" error="no such device" hardwareAddr="7e:06:77:f1:c6:7d" interface=veth0 ipAddr=9.9.9.250 subsys=node-neigh-debug
	 level=info msg="Unable to insert neighbor" error="no such device" hardwareAddr="7e:06:77:f1:c6:7d" interface=veth0 ipAddr=9.9.9.250 subsys=node-neigh-debug
	 START: node_linux_test.go:138: linuxPrivilegedIPv4OnlyTestSuite.TearDownTest
	 level=info msg="Unable to insert neighbor" error="no such device" hardwareAddr="7e:06:77:f1:c6:7d" interface=veth0 ipAddr=9.9.9.250 subsys=node-neigh-debug
	 PASS: node_linux_test.go:138: linuxPrivilegedIPv4OnlyTestSuite.TearDownTest	0.052s
	
	 FAIL: node_linux_test.go:956: linuxPrivilegedIPv4OnlyTestSuite.TestArpPingHandling

while the failure that was fixed was reporting:

	 node_linux_test.go:1207:
	     wait(nodev1.Identity(), nil, true)
	 node_linux_test.go:1048:
	     c.Assert(err, check.IsNil)
	 ... value *errors.errorString = &errors.errorString{s:"timeout reached while waiting for condition"} ("timeout reached while waiting for condition")
	 
	 START: node_linux_test.go:138: linuxPrivilegedIPv4OnlyTestSuite.TearDownTest
	 PASS: node_linux_test.go:138: linuxPrivilegedIPv4OnlyTestSuite.TearDownTest	0.052s
	 
	 FAIL: node_linux_test.go:956: linuxPrivilegedIPv4OnlyTestSuite.TestArpPingHandling

I filled #16573

@joestringer joestringer mentioned this pull request Jul 22, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jibi jibi jibi approved these changes

@brb brb brb approved these changes

@christarazi christarazi christarazi approved these changes

@aanm aanm aanm approved these changes

@jrajahalme jrajahalme jrajahalme approved these changes

@qmonnet qmonnet qmonnet approved these changes

@aditighag aditighag aditighag approved these changes

Assignees
No one assigned
Labels
kind/backports This PR provides functionality previously merged into master. ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@gandro @aditighag @jibi @brb @christarazi @aanm @jrajahalme @qmonnet @bmcustodio @h3llix