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.9 backports 2022-03-29 #19252

Merged
merged 3 commits into from Mar 30, 2022
Merged

v1.9 backports 2022-03-29 #19252

merged 3 commits into from Mar 30, 2022

Conversation

qmonnet
Copy link
Member

@qmonnet qmonnet commented Mar 29, 2022

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

$ for pr in 19053 19071 19193; do contrib/backporting/set-labels.py $pr done 1.9; done

or with

$ make add-label BRANCH=v1.9 ISSUES=19053,19071,19193

@qmonnet qmonnet requested a review from a team as a code owner March 29, 2022 11:03
@maintainer-s-little-helper maintainer-s-little-helper bot added backport/1.9 kind/backports This PR provides functionality previously merged into master. labels Mar 29, 2022
@qmonnet
Copy link
Member Author

qmonnet commented Mar 29, 2022

/test-backport-1.9

nathanjsweet and others added 3 commits March 29, 2022 12:12
[ upstream commit 0f4d3a7 ]

In October 2020, we made changes[1] to the cilium-agent's
ClusterRole to be more permissive. We did this, because
Openshift enables[2] the OwnerReferencesPermissionEnforcement[3]
admission controller. This admissions controller prevents
changes to the metadata.ownerReferences of any object
unless the entity (the cilium-agent in this case) has
permission to delete the object as well. Furthermore,
the controller allows protects metadata.ownerReferences[x].blockOwnerDeletion
of a resource unless the entity (again, the cilium-agent) has
"update" access to the finalizer of the object having its
deletion blocked. The original PR mistakenly assumed we
set ownerReferences on pods and expanded cilium-agent's
permissions beyond what was necessary. Cilium-agent
only sets ownerReferences on a CiliumEndpoint and the
blockOwnerDeletion field propagates up to the "owning"
pod of the endpoint. Cilium-agent only needs to be able
to delete CiliumEndpoints (which it has always been able to)
and "update" pod/finalizers (to set the blockOwnerDeletion field
on CiliumEndpoints). All other changes contained in #13369
were unnecessary.

1 #13369
2 https://docs.openshift.com/container-platform/4.6/architecture/admission-plug-ins.html
3 https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#ownerreferencespermissionenforcement

[ Backport notes: The files have been renamed:

    - install/kubernetes/cilium/templates/cilium-agent/clusterrole.yaml
      is, on v1.9:
      install/kubernetes/cilium/templates/cilium-agent-clusterrole.yaml

    - install/kubernetes/cilium/templates/cilium-preflight/clusterrole.yaml
      is, on v1.9:
      install/kubernetes/cilium/templates/cilium-preflight-clusterrole.yaml

  Additionally, we run the following:

      make -C install/kubernetes experimental-install quick-install

  and commit the changes. ]

Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
[ upstream commit 75f597b ]

The Clustermesh-APIServer creates a CiliumEndPoint and sets
a node as its ownerReference, also setting blockOwnerDeletion
to "true". If the OwnerReferencesPermissionEnforcement admission
controller is enabled (such as in environments like Openshift) then
the Clustermesh-APIServer will fail to create the CiliumEndPoint
as it has insufficient privileges to set blockOwnerDeletion of
a node. It needs to be able to "update" "nodes/finalizers" in order
to do this. See #19053 for more details and references.

Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
[ upstream commit 2efbdd6 ]

Local Redirect Policy (LRP) namespace needs to match
with the backend pods selected by the LRP.

This check was missing in the case where backend
pods are deployed after an LRP that selects them
was applied.

Added unit tests.

Reported-by: Joe Stringer <joe@covalent.io>
Signed-off-by: Aditi Ghag <aditi@cilium.io>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@qmonnet qmonnet force-pushed the pr/v1.9-backport-2022-03-29 branch from eec48ce to 4bc2901 Compare March 29, 2022 11:13
@qmonnet
Copy link
Member Author

qmonnet commented Mar 29, 2022

/test-backport-1.9

Job 'Cilium-PR-K8s-1.17-kernel-5.4' hit: #17617 (93.73% similarity)

Copy link
Member

@nathanjsweet nathanjsweet left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@qmonnet
Copy link
Member Author

qmonnet commented Mar 30, 2022

(VM provisioning failure)
/test-runtime-4.9

@qmonnet
Copy link
Member Author

qmonnet commented Mar 30, 2022

(Flake, see MLH's comment above)
/test-1.17-5.4

@qmonnet
Copy link
Member Author

qmonnet commented Mar 30, 2022

(VM provisioning failure)
/test-runtime-4.9

EDIT: Ah, but this has to be #18919

@qmonnet
Copy link
Member Author

qmonnet commented Mar 30, 2022

I double-checked the backport for Aditi's commit, which applied with no conflict.
All tests save runtime (#18919) are green, I consider this ready to merge.

@qmonnet qmonnet merged commit 6298c86 into v1.9 Mar 30, 2022
@qmonnet qmonnet deleted the pr/v1.9-backport-2022-03-29 branch March 30, 2022 13:35
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

3 participants