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

Add template to test ipv6 and dual stack with k8s CI versions #4086

Merged
merged 1 commit into from
Oct 12, 2023

Conversation

CecileRobertMichon
Copy link
Contributor

@CecileRobertMichon CecileRobertMichon commented Oct 3, 2023

What type of PR is this?

What this PR does / why we need it: Adds templates for ipv6 and dual stack using release-branch CI k8s versions so we can test various k8s release branches with CAPZ before releases to catch future regressions like kubernetes/kubernetes#120999.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

  • cherry-pick candidate

TODOs:

  • squashed commits
  • includes documentation
  • adds unit tests

Release note:

NONE

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 3, 2023
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 3, 2023
@codecov
Copy link

codecov bot commented Oct 3, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (1c9583c) 57.63% compared to head (51ce7cf) 57.63%.
Report is 8 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4086   +/-   ##
=======================================
  Coverage   57.63%   57.63%           
=======================================
  Files         188      188           
  Lines       19202    19202           
=======================================
  Hits        11067    11067           
  Misses       7505     7505           
  Partials      630      630           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@CecileRobertMichon
Copy link
Contributor Author

@nojnhuh the broken link error is coming from https://github.com/nojnhuh/cluster-api-provider-azure/tree/aso/azure/services/asogroups which is referred to in the ASO proposal. I'm guessing that branch no longer exists... Is there a good replacement for it?

@CecileRobertMichon
Copy link
Contributor Author

/test pull-cluster-api-provider-azure-ipv6-conformance-with-ci-artifacts

@k8s-ci-robot
Copy link
Contributor

@CecileRobertMichon: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test pull-cluster-api-provider-azure-build
  • /test pull-cluster-api-provider-azure-ci-entrypoint
  • /test pull-cluster-api-provider-azure-e2e
  • /test pull-cluster-api-provider-azure-e2e-aks
  • /test pull-cluster-api-provider-azure-test
  • /test pull-cluster-api-provider-azure-verify

The following commands are available to trigger optional jobs:

  • /test pull-cluster-api-provider-azure-apidiff
  • /test pull-cluster-api-provider-azure-apiversion-upgrade
  • /test pull-cluster-api-provider-azure-capi-e2e
  • /test pull-cluster-api-provider-azure-conformance
  • /test pull-cluster-api-provider-azure-conformance-ipv6-with-ci-artifacts
  • /test pull-cluster-api-provider-azure-conformance-with-ci-artifacts
  • /test pull-cluster-api-provider-azure-e2e-optional
  • /test pull-cluster-api-provider-azure-e2e-workload-upgrade
  • /test pull-cluster-api-provider-azure-windows-containerd-upstream-with-ci-artifacts
  • /test pull-cluster-api-provider-azure-windows-containerd-upstream-with-ci-artifacts-serial-slow

Use /test all to run the following jobs that were automatically triggered:

  • pull-cluster-api-provider-azure-apidiff
  • pull-cluster-api-provider-azure-apiversion-upgrade
  • pull-cluster-api-provider-azure-build
  • pull-cluster-api-provider-azure-capi-e2e
  • pull-cluster-api-provider-azure-ci-entrypoint
  • pull-cluster-api-provider-azure-conformance
  • pull-cluster-api-provider-azure-conformance-with-ci-artifacts
  • pull-cluster-api-provider-azure-e2e
  • pull-cluster-api-provider-azure-e2e-aks
  • pull-cluster-api-provider-azure-test
  • pull-cluster-api-provider-azure-verify
  • pull-cluster-api-provider-azure-windows-containerd-upstream-with-ci-artifacts

In response to this:

/test pull-cluster-api-provider-azure-ipv6-conformance-with-ci-artifacts

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@CecileRobertMichon
Copy link
Contributor Author

/test pull-cluster-api-provider-azure-conformance-ipv6-with-ci-artifacts

1 similar comment
@CecileRobertMichon
Copy link
Contributor Author

/test pull-cluster-api-provider-azure-conformance-ipv6-with-ci-artifacts

@nojnhuh
Copy link
Contributor

nojnhuh commented Oct 4, 2023

The link now magically works again. I'll think about what better link to keep in the doc.
sry

@CecileRobertMichon
Copy link
Contributor Author

/test pull-cluster-api-provider-azure-conformance-ipv6-with-ci-artifacts
/test pull-cluster-api-provider-azure-conformance-dual-stack-with-ci-artifacts

1 similar comment
@CecileRobertMichon
Copy link
Contributor Author

/test pull-cluster-api-provider-azure-conformance-ipv6-with-ci-artifacts
/test pull-cluster-api-provider-azure-conformance-dual-stack-with-ci-artifacts

@CecileRobertMichon
Copy link
Contributor Author

/test pull-cluster-api-provider-azure-conformance-ipv6-with-ci-artifacts
/test pull-cluster-api-provider-azure-conformance-dual-stack-with-ci-artifacts

@CecileRobertMichon
Copy link
Contributor Author

/retest

@CecileRobertMichon CecileRobertMichon changed the title [WIP] Add template to test ipv6 + ci k8s versions Add template to test ipv6 and dual stack with k8s CI versions Oct 9, 2023
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 9, 2023
@CecileRobertMichon
Copy link
Contributor Author

/test pull-cluster-api-provider-azure-conformance-ipv6-with-ci-artifacts
/test pull-cluster-api-provider-azure-conformance-dual-stack-with-ci-artifacts

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Oct 9, 2023
@CecileRobertMichon
Copy link
Contributor Author

/test pull-cluster-api-provider-azure-conformance-ipv6-with-ci-artifacts
/test pull-cluster-api-provider-azure-conformance-dual-stack-with-ci-artifacts

@CecileRobertMichon
Copy link
Contributor Author

This is ready for review

/assign @nawazkh @jackfrancis

@CecileRobertMichon
Copy link
Contributor Author

/test pull-cluster-api-provider-azure-conformance-ipv6-with-ci-artifacts
/test pull-cluster-api-provider-azure-conformance-dual-stack-with-ci-artifacts

@CecileRobertMichon
Copy link
Contributor Author

CecileRobertMichon commented Oct 10, 2023

/hold cancel

rebase is done

@CecileRobertMichon
Copy link
Contributor Author

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 10, 2023
@CecileRobertMichon
Copy link
Contributor Author

/test pull-cluster-api-provider-azure-conformance-ipv6-with-ci-artifacts
/test pull-cluster-api-provider-azure-conformance-dual-stack-with-ci-artifacts

@CecileRobertMichon
Copy link
Contributor Author

/retest

@willie-yao
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 10, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 128e0ce72dab03bcf7d89e8989d2d6382bbd8e5c

@nawazkh
Copy link
Member

nawazkh commented Oct 10, 2023

/lgtm

@CecileRobertMichon
Copy link
Contributor Author

/assign @jackfrancis @mboersma

@@ -0,0 +1,12 @@
ginkgo.focus: \[Feature\:Networking-IPv6\]
ginkgo.skip: \[Feature\:SCTPConnectivity\]|\[Experimental\]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we skipping [Experimental] in ipv6 but not dual-stack? Do [Experimental]-tagged test scenarios require ipv4, and our k8s dual-stack clusters are configured in such a way that those ipv4-dependent tests can be reliably tested?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because there is no [Experimental] dual-stack test but there was one ipv6 one that was failing https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_cluster-api-provider-azure/4086/pull-cluster-api-provider-azure-conformance-dual-stack-with-ci-artifacts/1711771121797304320

I could add it to both but the dual-stack one would be a no-op, with ginkgo skip I prefer to only skip tests only as needed.

CI_URL="https://storage.googleapis.com/k8s-release-dev/ci/$${CI_VERSION}/bin/linux/amd64"
for CI_PACKAGE in "$${PACKAGES_TO_TEST[@]}"; do
echo "* downloading binary: $$CI_URL/$$CI_PACKAGE"
wget --inet4-only "$$CI_URL/$$CI_PACKAGE" -nv -O "$$CI_DIR/$$CI_PACKAGE"
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume that this --inet4-only flag has nothing to do w/ ipv4 k8s clusters (sort of a funny coincidence that this change would land in this PR :)).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does, this is to ensure we always connect to the ipv4 IP address of https://storage.googleapis.com/k8s-release-dev, IPv6 one was not reachable. It's a no-op for the ivp4 template since we always used the ipv4 adress but it ensure the ipv6/dual stack templates which are running on dual stack hosts use the ipv4 address.

Copy link
Contributor

Choose a reason for hiding this comment

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

ACK, was a little strange at first to see this in the ipv6 template, but it makes sense that the actual OS is dual stack (ipv6-only config is just the k8s surface area)

Copy link
Contributor

@jackfrancis jackfrancis left a comment

Choose a reason for hiding this comment

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

/lgtm

unless any of the questions I posed are actionable

@CecileRobertMichon
Copy link
Contributor Author

@jackfrancis can you please approve if all your comments were addressed?

@CecileRobertMichon
Copy link
Contributor Author

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 12, 2023
@k8s-ci-robot k8s-ci-robot merged commit 85c23aa into kubernetes-sigs:main Oct 12, 2023
11 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.12 milestone Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants