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.7 backports 2020-04-02 #10833

Merged
merged 9 commits into from
Apr 3, 2020
Merged

v1.7 backports 2020-04-02 #10833

merged 9 commits into from
Apr 3, 2020

Conversation

pchaigno
Copy link
Member

@pchaigno pchaigno commented Apr 2, 2020

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

$ for pr in 10717 10727 10782; do contrib/backporting/set-labels.py $pr done 1.7; done

pchaigno and others added 5 commits April 2, 2020 15:26
[ upstream commit d12853f ]

After each test case, we retrieve Cilium logs to look for known-bad log
messages. Daniel recently noticed that tests containing known-bad
message logs are passing.

The issue is in the kubectl logs command we use to retrieve the logs.
Its behavior seems to have changed at some point. When used with a label
selector---as we do---the option --tail has a default value of 10
instead of -1. We are therefore only seeing the 10 last lines of each
Cilium pod, regardless of the --since option's value.

Reported-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Paul Chaignon <paul@cilium.io>
[ upstream commit f0049da ]

CNP schema validation was incorrectly formatted for some fields which
could cause badly formatted yaml files to be accepted by kube-apiserver
bypassing the schema validation. This would later cause Cilium to print
errors and potentially avoid it from starting, as the invalid CNPs would
prevent Cilium from fully synchronize with kube-apiserver an operation
that is essential when starting Cilium.

This commit fixes all violations presented by kubernetes for the CCNP
and CNP validation which would then deny bad CNPs and / or CCNPs from
being accepted by kube-apiserver.

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Paul Chaignon <paul@cilium.io>
[ upstream commit ff863a8 ]

As Cilium might require to update its CRD validation schema it is
important for the users to make sure all policies installed in their
cluster are valid in the point of view the new CRD validation schema
before performing an upgrade.

Avoiding doing this validation might cause Cilium from updating its
NodeStatus in those invalid Network Policies as well as in the worst
case scenario it might give a false sense of security to the user if
a policy is badly formatted and Cilium is not inforcing that policy
due a bad validation.

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Paul Chaignon <paul@cilium.io>
[ upstream commit 6a9ceba ]

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Paul Chaignon <paul@cilium.io>
[ upstream commit bb9c843 ]

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno pchaigno added requires-janitor-review kind/backports This PR provides functionality previously merged into master. labels Apr 2, 2020
@pchaigno pchaigno requested a review from a team as a code owner April 2, 2020 16:09
@aanm
Copy link
Member

aanm commented Apr 2, 2020

never-tell-me-the-odds

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.

My commits LGTM

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

@joestringer
Copy link
Member

Legitimate failure by the looks:

daemon/cmd/nodeport_linux_test.go:83:9: undefined: checkNodePortAndEphemeralPortRanges
	 make: *** [tests-privileged] Error 1

@joestringer joestringer mentioned this pull request Apr 2, 2020
27 tasks
@brb
Copy link
Member

brb commented Apr 2, 2020

Re undefined: checkNodePortAndEphemeralPortRanges: the problem is that the failing test is in the daemon/cmd dir, while the function is in the daemon/. Perhaps we should move the tests from daemon/cmd to daemon/, or maybe the function public. I'd vote for the former.

brb added 3 commits April 2, 2020 22:26
[ upstream commit a2d6217 ]

This commit removes the ephemeral port range check for NodePort from the
BPF datapath.

Instead, in the agent we check whether the NodePort range is covered by
ip_local_reserved_ports. If it's not, then we append the range to
ip_local_reserved_ports. Users can opt out from the latter by setting
--enable-auto-protect-node-port-range=false.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Paul Chaignon <paul@cilium.io>
[ upstream commit a064bf5 ]

The value configures --enable-auto-protect-node-port-range. The default
is true.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Paul Chaignon <paul@cilium.io>
[ upstream commit 9b8f7fc ]

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno
Copy link
Member Author

pchaigno commented Apr 2, 2020

Thanks @brb! I went with the former and moved nodeport_linux_test.go to daemon/.

@joestringer
Copy link
Member

@pchaigno I don't think that github registered the new version.

@pchaigno
Copy link
Member Author

pchaigno commented Apr 3, 2020

@joestringer Uh, you're right. Nonetheless:

$ git push -f origin pr/v1.7-backport-2020-04-02 
Everything up-to-date

🤔

EDIT: I changed the commit date of the last commit and that worked.

[ upstream commit 8162f6b ]

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno pchaigno force-pushed the pr/v1.7-backport-2020-04-02 branch from 90a363f to 3c3592b Compare April 3, 2020 06:04
@pchaigno
Copy link
Member Author

pchaigno commented Apr 3, 2020

Cilium-kubernetes-upstream-test hit a timeout error.

test-upstream-k8s

@pchaigno
Copy link
Member Author

pchaigno commented Apr 3, 2020

test-missed-k8s

Hit #10231.

@pchaigno
Copy link
Member Author

pchaigno commented Apr 3, 2020

restart-ginkgo

Hit #10636.

@joestringer
Copy link
Member

test-with-kernel

@joestringer
Copy link
Member

(test-with-kernel shows no pipeline steps, not sure if that's just because the target is unsupported on v1.7 branch.. https://jenkins.cilium.io/job/Cilium-PR-Ginkgo-Tests-Kernel/140/flowGraphTable/)

@joestringer
Copy link
Member

OK, 1.7 just doesn't support that target. I'll merge this now.

@joestringer joestringer merged commit e0945c8 into v1.7 Apr 3, 2020
@joestringer joestringer deleted the pr/v1.7-backport-2020-04-02 branch April 3, 2020 16:48
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

4 participants