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.10 backports 2021-06-01 #16384

Merged
merged 33 commits into from
Jun 4, 2021
Merged

v1.10 backports 2021-06-01 #16384

merged 33 commits into from
Jun 4, 2021

Conversation

qmonnet
Copy link
Member

@qmonnet qmonnet commented Jun 1, 2021 •
edited by gandro

Skipped:

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

$ for pr in 16052 16126 16263 16274 16293 16081 16268 16288 16310 16211 16286 16271 16227 16176 16043 16137 16194 16319 16325 16055 12527 16336 16312; do contrib/backporting/set-labels.py $pr done 1.10; done

@qmonnet qmonnet requested review from a team as code owners June 1, 2021 15:32
@qmonnet qmonnet requested review from nathanjsweet and aanm June 1, 2021 15:32
@qmonnet qmonnet added backport/1.10 kind/backports This PR provides functionality previously merged into master. labels Jun 1, 2021
@qmonnet qmonnet mentioned this pull request Jun 1, 2021
Closed
pchaigno
pchaigno approved these changes Jun 1, 2021
View reviewed changes
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 PRs look good.

ungureanuvladvictor
ungureanuvladvictor approved these changes Jun 1, 2021
View reviewed changes
Copy link
Member

@ungureanuvladvictor ungureanuvladvictor left a comment

Choose a reason for hiding this comment

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

My Azure PRs look good.

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

And my axe!

@qmonnet
Copy link
Member Author

qmonnet commented Jun 1, 2021

test-backport-1.10

aanm
aanm approved these changes Jun 2, 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 commits

@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 2, 2021
@h3llix
Copy link
Contributor

h3llix commented Jun 2, 2021

Does everyone need to approve their commits in such backport pr's ?

@pchaigno
Copy link
Member

pchaigno commented Jun 2, 2021

Does everyone need to approve their commits in such backport pr's ?

@h3llix Good question. It's best if all authors review, but we typically only make sure that backported commits that had conflicts are reviewed. The other commits typically matter a bit less (though that doesn't mean they can't break something).

I'm not sure the backporting documentation makes that clear. If it's not the case, we should update it.

@nathanjsweet nathanjsweet added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jun 2, 2021
@qmonnet qmonnet removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 2, 2021
@qmonnet qmonnet force-pushed the pr/v1.10-backport-2021-06-01 branch from 1e789d0 to 6db5355 Compare June 3, 2021 08:57
@qmonnet qmonnet removed the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jun 3, 2021
@qmonnet
Copy link
Member Author

qmonnet commented Jun 3, 2021

I simply rebased on top of de53e80. No change to backported commits.

@pchaigno
Copy link
Member

pchaigno commented Jun 3, 2021

test-backport-1.10

@nathanjsweet
Copy link
Member

I'm seeing a lot of failures for k8s1.17-kernel-4.9

@qmonnet
Copy link
Member Author

qmonnet commented Jun 3, 2021 •
edited

Right, there are some flakes:

As for k8s-1.17-kernel-4.9, there are 11 tests failing, and I've not managed yet to figure out if there is a root cause or if they might be independent.

jrajahalme and others added 21 commits June 4, 2021 10:05
@jrajahalme @qmonnet
envoy: Remove request headers from response access logs
beea9d2 
[ upstream commit 415c624 ]

Remove request headers from response access logs, except for
'x-request-id', which is retained for request/response correlation
purposes.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@aanm @qmonnet
pkg/k8s: set right logic to ignore ipcache errors
99d1631 
[ upstream commit e6ae6fb ]

The logic to ignore errors is inverted and the errors are still being
printed as warnings. This commit inverts the logic so that only relevant
warnings are printed.

Fixes: 0ab4fa1 ("pkg/k8s: ignore certain ipcache errors")
Fixes: 465cac1 ("pkg/k8s: ignore overwrite source "custom-resource" with "k8s" errors")
Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@jrajahalme @qmonnet
endpoint: Skip waiting-to-regenerate -> waiting-for-identity transitions
6b64530 
[ upstream commit 1e5f74d ]

Regeneration logic fails if waiting-for-identity changes to ready
state in a scenario like this:

  builder: ready -> waiting-to-regenerate
  ..
  label change etc: waiting-to-regenerate -> waiting-for-identity
  ..
  labels resolved: waiting-for-identity -> ready
  ..
  builder: (ready) -> regenerating (FAILS as this is not expected)

Resolve this by giving precedence to the waiting-to-regenerate state
over the waiting-for-identity state.  Compensate for possibly blocking
this state change in Cilium endpoint PATCH API.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@pchaigno @qmonnet
loader: Revert removal of endpoint routes
c6b4ac0 
[ upstream commit f937df7 ]

This commit is a partial revert of 72e6238 ("loader: Remove program and
route when disable endpoint routes").

Commit 72e6238 started removing existing endpoint routes when
enable-endpoint-routes is disabled in the agent. In chaining mode
however, if Cilium isn't the primary CNI, it isn't responsible for
the endpoint's networking. In that case, the primary CNI may install
and rely on those endpoint routes and we shouldn't remove them.

This commit reverts the removal of endpoint routes. We'll provide a
proper solution to remove only endpoint routes Cilium "owns" in a
subsequent commit.

Fixes: 72e6238 ("loader: Remove program and route when disable endpoint routes")
Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@pchaigno @qmonnet
Revert "endpoint: Overwrite endpoint datapath config. on restore"
9d410f2 
[ upstream commit 320ea0d ]

This commit partially reverts commit a9ecab1.

Disabling endpoint routes in an existing cluster is not supported for
now. We first need to find a way to properly remove the endpoint routes
(see previous commit) before we can support this.

We keep the override of endpoint datapath config. for the host endpoint
as otherwise host firewall test will error due to a failure to load
bpf_host.

Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@pchaigno @qmonnet
test/K8sCustomCalls: Deploy pods after Cilium only
62762e6 
[ upstream commit db93de5 ]

We need to deploy pods after Cilium is installed or they may receive the
datapath corresponding to a previous Cilium installation.

Fixes: 37f6192 ("test: add CI test for tail calls hooks for custom programs")
Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@pchaigno @qmonnet
Revert "endpoint: Refactor init of EndpointDatapathConfiguration"
154c791 
[ upstream commit 8da8b88 ]

Commit 0875453 ("endpoint: Refactor init of
EndpointDatapathConfiguration") leads to .RequireEgressProg being
overwritten on endpoint creation. That in turns breaks reverse NAT when
running in chaining mode [1].

This commit is a partial revert of commit
0875453, keeping only a helper function.

1 - https://github.com/cilium/cilium/blob/v1.10.0/plugins/cilium-cni/chaining/generic-veth/generic-veth.go#L165
Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@aanm @qmonnet
contrib: simplify check-docker-images script
0df8090 
[ upstream commit 8e1ef9c ]

The curl URL fails if the sha256 is no longer part of the tag. Running
with `docker buildx imagetools inspect` it is possible to verify if an
image digest exists regardless even if no longer belongs to a tag.

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@ungureanuvladvictor @qmonnet
Bump azure vendor
42c7896 
[ upstream commit 943f792 ]

github.com/Azure/azure-sdk-for-go/services/compute -> 2021-03-01
github.com/Azure/azure-sdk-for-go/services/network -> 2020-11-01

[ backport note:
    Update azure vendor from v50.0.0+incompatible instead of
    v50.2.0+incompatible as for master branch. ]

Signed-off-by: Vlad Ungureanu <vladu@palantir.com>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@ungureanuvladvictor @qmonnet
Support custom Azure clouds
76c5691 
[ upstream commit 34424b0 ]

This commit swaps the creation of Azure service clients to use the Azure
Resource Manager baseURI which points to the right Azure cloud extracted
from the environment.

Signed-off-by: Vlad Ungureanu <vladu@palantir.com>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@qmonnet
docs: document the policy for backporting documentation changes
025ac5c 
[ upstream commit 2a356d9 ]

Documentation changes should be backported "as far as they go" on the
supported branches, so that users can get relevant information from the
documentation branch associated to the software version they run.
Document this as part as the criteria for backports.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@aditighag @qmonnet
test: Don't keep (hard-coded) wait group count
751b794 
[ upstream commit 1d8f8e2 ]

This can be error-prone, and unnecessary.

Fixes: 7387ca2
Signed-off-by: Aditi Ghag <aditi@cilium.io>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@aditighag @qmonnet
test: Make LRP restore test case robust and optimized
c6c9790 
[ upstream commit 6a3e846 ]

The goal of the test is to check if curl to a clusterIP svc endpoint is redirected
to both the backends when the original svc entry is restored upon LRP removal.
However, the current test logic expects the same backend should be selected for
all the pod clients simultaneously, and this can lengthen test duration.
This doesn't seem right since backend selection is not exactly deterministic.
More importantly, we only need both backends to be selected at least once for all the client pods.

Flip the order in which we loop over backends and client pods. Loop over client pods first, and
then make curl calls until we hit both the backends on each of the client pods.
This way we can potentially avoid making some of the curl duplicate calls by not having
to synchronize what backends VIP calls are redirected to across multiple nodes.

Signed-off-by: Aditi Ghag <aditi@cilium.io>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@pchaigno @qmonnet
.github: Don't wait for GKE cluster cleanup
fd06534 
[ upstream commit 689a725 ]

In commit ba737f3 (".github: Parallelize cleanup of multicluster
setup"), I missed that gcloud can also take an --async flag to not wait
for the actual cluster deletion to happen and return immediately
(similarly to AKS' --no-wait).

This commit therefore reverts ba737f3 (".github: Parallelize cleanup
of multicluster setup") and makes use of --async instead.

Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@netflash @qmonnet
typo
5e32a73 
[ upstream commit a0e3269 ]

Signed-off-by: Alex Romanov <alex@romanov.ws>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@h3llix @qmonnet
bugtool: add missing bpftool map dumps
5651f88 
[ upstream commit c573ff8 ]

Fixes:cilium#16008

Signed-off-by: Gaurav Genani <h3llix.pvt@gmail.com>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@christarazi @qmonnet
Makefile, contrib: Add script to create kind cluster
11e1a70 
[ upstream commit ff9be87 ]

We have the equivalent for minikube.

Signed-off-by: Chris Tarazi <chris@isovalent.com>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@aanm @qmonnet
docs: fix some dead links
2463f91 
[ upstream commit c97f353 ]

Replaced some dead links with alive ones.

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@aanm @qmonnet
.github: rename workflow files
a9b29a5 
[ upstream commit 38f0af9 ]

Add prefix accordingly their main usage.

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@aanm @qmonnet
.github: pin SHAs for all GitHub actions
d119501 
[ upstream commit 948bb90 ]

[ backport note:
    Update action in documentation.yaml from actions/checkout@v1 instead
    of actions/checkout@v2 as for master branch. ]

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@aanm @qmonnet
.github: refactored base image lint GH action
c447f76 
[ upstream commit 42a370b ]

Refactored this GH action and remove unnecessary triggers for it.

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@qmonnet qmonnet force-pushed the pr/v1.10-backport-2021-06-01 branch from 6db5355 to c447f76 Compare June 4, 2021 09:10
@qmonnet
Copy link
Member Author

qmonnet commented Jun 4, 2021

New conflict, new rebase.
Still looking at the previous failure, but so far I think they might have been independent failures, documented in existing PRs.
I'll re-run the tests with the latest rebase anyway.

@qmonnet
Copy link
Member Author

qmonnet commented Jun 4, 2021

test-backport-1.10

@qmonnet
Copy link
Member Author

qmonnet commented Jun 4, 2021 •
edited

Some failures again, but all identifiable as documented flakes.

@nathanjsweet nathanjsweet added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 4, 2021
@nathanjsweet nathanjsweet merged commit c1c7f53 into cilium:v1.10 Jun 4, 2021
@qmonnet qmonnet deleted the pr/v1.10-backport-2021-06-01 branch June 4, 2021 16:49
@nathanjsweet nathanjsweet mentioned this pull request Jun 4, 2021
Merged
@aanm aanm mentioned this pull request Jun 16, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gandro gandro gandro approved these changes

@pchaigno pchaigno pchaigno approved these changes

@ungureanuvladvictor ungureanuvladvictor ungureanuvladvictor approved these changes

@christarazi christarazi christarazi approved these changes

@nbusseneau nbusseneau nbusseneau approved these changes

@aanm aanm aanm approved these changes

@michi-covalent michi-covalent michi-covalent approved these changes

@nathanjsweet nathanjsweet Awaiting requested review from nathanjsweet

Assignees

@nathanjsweet nathanjsweet

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