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

ci:[CNI] Windows datapath using k8se2e template #2041

Merged
merged 5 commits into from
Jul 28, 2023

Conversation

jpayne3506
Copy link
Contributor

@jpayne3506 jpayne3506 commented Jul 5, 2023

Reason for Change:

Provides windows datapath tests through k8s e2e conformance testing to the load test pipeline.

Issue Fixed:

Requirements:

Notes:
PM for documented test coverage and ADO runs.
Will need to be updated after #2032 merges as file names will be changing for windows go tests.

@jpayne3506 jpayne3506 added cni Related to CNI. ci Infra or tooling. labels Jul 5, 2023
@jpayne3506 jpayne3506 requested review from vipul-21 and paulyufan2 July 5, 2023 18:01
@jpayne3506 jpayne3506 self-assigned this Jul 5, 2023
@jpayne3506 jpayne3506 changed the title Jpayne3506/k8se2e datapath ci:[CNI] Windows datapath pipeline template Jul 5, 2023
parameters:
clusterName: ""

steps:
Copy link
Contributor

Choose a reason for hiding this comment

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

is there actually anything windows specific about this or could we reuse this template across all scenarios?

Copy link
Contributor Author

@jpayne3506 jpayne3506 Jul 5, 2023

Choose a reason for hiding this comment

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

Specifically, lines 45 and 47 are what is making this into a windows file. After looking into this I could add parameters to the load-test-template that calls it to ensure that this could be used for other k8s e2e.test testing. Thus, making it a better template.

@jpayne3506 jpayne3506 force-pushed the jpayne3506/k8se2e-datapath branch 15 times, most recently from 1bbbb0c to ffea64a Compare July 7, 2023 21:14
@jpayne3506 jpayne3506 requested a review from rbtr July 7, 2023 21:14
@jpayne3506 jpayne3506 changed the title ci:[CNI] Windows datapath pipeline template ci:[CNI] Windows datapath using k8se2e pipeline template Jul 7, 2023
@jpayne3506 jpayne3506 force-pushed the jpayne3506/k8se2e-datapath branch 5 times, most recently from e749438 to 06c324c Compare July 7, 2023 21:24
@jpayne3506 jpayne3506 marked this pull request as ready for review July 7, 2023 21:24
@jpayne3506 jpayne3506 requested a review from a team as a code owner July 7, 2023 21:24
rbtr
rbtr previously approved these changes Jul 10, 2023
@@ -16,7 +16,7 @@ spec:
spec:
containers:
- name: noop
image: mcr.microsoft.com/windows/nanoserver:ltsc2022
image: mcr.microsoft.com/oss/kubernetes/pause:3.6
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of just changing this image, should evaluate if this deployment needs to be OS-specific or if there should only be one using (this) multiplat image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will test on a separate branch and communicate with @vipul-21 to see what we should do going forward.

vipul-21
vipul-21 previously approved these changes Jul 11, 2023
# Needs to be changed after #2032 is merged
# test/integration/datapath/datapath_windows_test.go
# does not test IPv6 at this time, waiting for #2032
go test -count=1 test/integration/datapath/datapath_win_test.go -timeout 3m -tags connection -run ^TestDatapathWin$ -tags=connection
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a make command for this ? Because we will be adding this to the PR pipeline and any future change would be easy to make.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#2086 - Constrained to only network conformance, but very useful one line command.

@jpayne3506 jpayne3506 dismissed stale reviews from vipul-21 and rbtr via e749438 July 12, 2023 01:01
@jpayne3506 jpayne3506 force-pushed the jpayne3506/k8se2e-datapath branch from 2a9a1b3 to e749438 Compare July 12, 2023 01:01
@jpayne3506 jpayne3506 force-pushed the jpayne3506/k8se2e-datapath branch from d01b91b to 800331c Compare July 28, 2023 01:16
@jpayne3506 jpayne3506 changed the title ci:[CNI] Windows datapath using k8se2e pipeline template ci:[CNI] Windows datapath using k8se2e template Jul 28, 2023
Copy link
Contributor

@paulyufan2 paulyufan2 left a comment

Choose a reason for hiding this comment

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

lgtm

@jpayne3506 jpayne3506 force-pushed the jpayne3506/k8se2e-datapath branch from df5cd25 to d6c939b Compare July 28, 2023 17:06
jobs:
- template: ../k8s-e2e/k8s-e2e-job-template.yaml
parameters:
sub: $(TEST_SUB_SERVICE_CONNECTION)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to send the sub as a parameter to the templates ?

@jpayne3506 jpayne3506 merged commit 3658951 into master Jul 28, 2023
@jpayne3506 jpayne3506 deleted the jpayne3506/k8se2e-datapath branch July 28, 2023 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Infra or tooling. cni Related to CNI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants