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.10 backports 2021-10-28 #17727

Merged
merged 5 commits into from
Nov 8, 2021
Merged

Conversation

nebril
Copy link
Member

@nebril nebril commented Oct 28, 2021

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

$ for pr in 17402 17407 17616 17630 17708; do contrib/backporting/set-labels.py $pr done 1.10; done

or with

$ make add-label branch=v1.10 issues=17402,17407,17616,17630,17708

michi-covalent and others added 4 commits October 28, 2021 11:08
[ upstream commit 339baa8 ]

Python sysdump has been deprecated.

Ref: https://github.com/cilium/cilium-sysdump

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
[ upstream commit f1463a8 ]

The `pkg/monitor/agent` package mainly performs three tasks:

 - Managing subscribers of monitor events (such as `cilium monitor`
   clients or the Hubble observer)
 - Receiving and broadcasting user-space notifications ("agent events")
   to the above subscribers.
 - Polling the perf event ring buffer for datapath notifications and
   broadcasting them to subscribers.

The third task can only be performed once the perf event ring buffer map
has been set up by the daemon. However, some subsystems will try to send
user-space notifications before the events map has been set up. This
often causes an unnecessary log warning to be emitted, which regularly
confuses users (see cilium#14146). Fundamentally however, there is no reason
to drop user-space events if the datapath is not set up yet.

Therefore, this commit splits the monitor agent constructor into two
functions: The first one, `NewAgent` initializes the subscriber
management and allows user-space notifications to be received early, the
second function `AttachToEventsMap` opens the perf event ring buffer to
poll datapath events.

By splitting up the initialization phase and invoking the constructor
earlier, we can avoid the data race in cilium#17167 and we also avoid emitting
a unhelpful warning if user-space events are created early.

Note: Because subscribers (i.e. Hubble or `cilium monitor` clients) are
still attached relatively late, we will still drop early events if there
is no one listening for them. This commit does not address that - but
instead of emitting a confusing warning that the user cannot do anything
about, it will drop the event the same way Cilium up to version 1.8 did.

Fixes: cilium#17167

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
[ upstream commit f5a1621 ]

This change refactors and names 3 of `cmd/daemon` `init` functions so
that the order of their execution and what they do is clearer.

Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
[ upstream commit 9a9e471 ]

This commit does not introduce any functional change, it just renames
the package to make it explicit it relates to the egress gateway
feature.

Fixes: cilium#17389
Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
@nebril nebril requested a review from a team as a code owner October 28, 2021 09:11
@maintainer-s-little-helper maintainer-s-little-helper bot added backport/1.10 kind/backports This PR provides functionality previously merged into master. labels Oct 28, 2021
Copy link
Member

@gandro gandro left a comment

Choose a reason for hiding this comment

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

My commit looks good! Thanks!

@nebril
Copy link
Member Author

nebril commented Oct 28, 2021

/test-backport-1.10

Job 'Cilium-PR-K8s-1.18-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.18-kernel-4.9 so I can create a new GitHub issue to track it.

@nebril
Copy link
Member Author

nebril commented Oct 28, 2021

restarting net-next because of infra issue

@nebril
Copy link
Member Author

nebril commented Oct 28, 2021

test-1.16-netnext

[ upstream commit 7db6998 ]

Suppress warning message from helm:

warning: skipped value for l7Proxy: Not a table.

Signed-off-by: Chen Yaqi <chendotjs@gmail.com>
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
@nebril nebril force-pushed the pr/v1.10-backport-2021-10-28 branch from 77e79ba to 46eb254 Compare October 29, 2021 08:56
@nebril
Copy link
Member Author

nebril commented Oct 29, 2021

repushed to get l4lb to use updated workflow file

@nebril
Copy link
Member Author

nebril commented Oct 29, 2021

/test-backport-1.10

@nebril
Copy link
Member Author

nebril commented Oct 30, 2021

test-1.16-netnext

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.

Those PRs look good to me 👍

@pchaigno
Copy link
Member

pchaigno commented Nov 5, 2021

Previous run had a timeout, retriggering and will check if it's consistent over time.
test-1.16-netnext

@pchaigno
Copy link
Member

pchaigno commented Nov 5, 2021

Weird VirtualBox provisioning error :-/
test-1.16-netnext

@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 Nov 5, 2021
@pchaigno pchaigno merged commit cd6c48a into cilium:v1.10 Nov 8, 2021
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. ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants