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.13 Backports 2023-12-13 #29847

Merged
merged 3 commits into from
Dec 13, 2023
Merged

v1.13 Backports 2023-12-13 #29847

merged 3 commits into from
Dec 13, 2023

Conversation

giorio94
Copy link
Member

@giorio94 giorio94 commented Dec 13, 2023

Once this PR is merged, a GitHub action will update the labels of these PRs:

 29455 29694 29675

mhofstetter and others added 3 commits December 13, 2023 11:07
[ upstream commit a2694fc ]

[ backporter's notes: skipped changes in conformance-ginkgo.yaml,
  conformance-runtime.yaml, tests-datapath-verifier.yaml and
  tests-e2e-upgrade.yaml, as not present in v1.13. ]

The property dns-resolver has been removed from the little-vm-helper
GitHub action.

Therefore, this commit removes the usage of it in the cilium repository.

See: cilium/little-vm-helper#118

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 1f1c384 ]

Cloud provider related workflows use the full configuration as matrix
when being executed on a scheduled basis on stable branches, whereas
only the default configuration is used on PR workflows.

Currently, this decision checks whether the workflow is triggered
via `event_name == schedule`.

This is working fine on `main`, but not on all other stable branches
where the workflows are triggered via workflow_dispatch event (called
by a scheduled workflow (ariane-scheduled.yaml) on main).

Therefore, this commit extends the decision to check for the input `PR-number`
starting with a "v". This is the case for Ariane triggered runs - as they pass
the branch name as PR-number (PR runs pass the actual numeric PR number).

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit dbe56dd ]

Now that known issues causing connection disruption (which appeared to
mostly affect dual stack clusters) have been fixed, let's enable IPv6
again in the clustermesh upgrade/downgrade workflow.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
@giorio94 giorio94 added kind/backports This PR provides functionality previously merged into master. backport/1.13 This PR represents a backport for Cilium 1.13.x of a PR that was merged to main. labels Dec 13, 2023
@giorio94 giorio94 requested review from mhofstetter and qmonnet and removed request for qmonnet December 13, 2023 10:21
Copy link
Member

@mhofstetter mhofstetter 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 look good. Thanks @giorio94

@giorio94
Copy link
Member Author

giorio94 commented Dec 13, 2023

@mhofstetter It seems to me that 010f9ee may break CI, because some of the images have been created before cilium/little-vm-helper-images#187 was merged. In particular, bpf-next ones, which are not bumped automatically. I initially thought we needed to bump the action version (and opened #29850), but that is not enough as the images themselves need to be updated. On the contrary, bumping the action may break CI as well, due to the removal of the dns-resolver parameter.

If this matches your understanding, do you have any ideas how to address it? /cc @brb Would it be possible upgrading the bpf-next images, or should we stick to the previous LVH action version?

@mhofstetter
Copy link
Member

@mhofstetter It seems to me that 010f9ee may break CI, because some of the images have been created before cilium/little-vm-helper-images#187 was merged. In particular, bpf-next ones, which are not bumped automatically. I initially thought we needed to bump the action version (and opened #29850), but that is not enough as the images themselves need to be updated. On the contrary, bumping the action may break CI as well, due to the removal of the dns-resolver parameter.

If this matches your understanding, do you have any ideas how to address it? /cc @brb Would it be possible upgrading the bpf-next images, or should we stick to the previous LVH action version?

@giorio94 Good point. But potential issues should be detected when executing the CI test runs. If so, I agree that we should skip this backport and update the LVH images first.

As for breaking CI when upgrading the action: This shouldn't be the case because the use of undefined parameters only results in warnings. This was also the original reason why @tklauser added the backport labels for the related PR: #29455 (comment)

@giorio94
Copy link
Member Author

But potential issues should be detected when executing the CI test runs.

Yeah, let's run it and decide based on the outcome.

This shouldn't be the case because the use of undefined parameters only results in warnings.

Sure. What I meant is that if this one breaks CI, that would as well, because the dns-parameter would not be configured anymore.

@giorio94
Copy link
Member Author

/test-backport-1.13

@giorio94
Copy link
Member Author

But potential issues should be detected when executing the CI test runs.

Yeah, let's run it and decide based on the outcome.

GHA-based CI looks green (except for the ipsec known flake) 🚀

@giorio94 giorio94 marked this pull request as ready for review December 13, 2023 15:08
@giorio94 giorio94 requested review from a team as code owners December 13, 2023 15:08
@giorio94
Copy link
Member Author

Conformance GKE failed due to node churn caused by spot instances. Rerunning

@giorio94
Copy link
Member Author

/test-1.24-4.19

@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 Dec 13, 2023
@giorio94
Copy link
Member Author

Cilium IPsec upgrade failure is a known flake, and the check is not marked as required: #29702. This is good to be merged.

@ldelossa ldelossa merged commit 78fed70 into v1.13 Dec 13, 2023
133 of 136 checks passed
@ldelossa ldelossa deleted the pr/v1.13-backport-2023-12-13 branch December 13, 2023 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.13 This PR represents a backport for Cilium 1.13.x of a PR that was merged to main. 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
No open projects
Status: Released
Development

Successfully merging this pull request may close these issues.

None yet

4 participants