Skip to content

Conversation

@camrynl
Copy link
Contributor

@camrynl camrynl commented Sep 28, 2022

Reason for Change:

Current state of e2e tests changes to dropgz/azure-ipam on every run regardless of whether or not change is present.
Adding submodule pipeline that will only trigger runs if there are changes in the azure-ipam or dropgz directories. This pipeline will test the cni-dropgz-test image.
When changes are not present, the e2e tests will run only on the regular cni-dropgz image, which is built from the cni installer that utilizes released versions of components.

New Submodule Pipeline: https://dev.azure.com/msazure/One/_build?definitionId=292133&_a=summary

Issue Fixed:

Requirements:

Notes:

@camrynl
Copy link
Contributor Author

camrynl commented Oct 4, 2022

validated pipeline tests via manual run: https://dev.azure.com/msazure/One/_build/results?buildId=61311663&view=results

unable to run the full submodule pipeline via PR trigger since submodules-pipeline.yaml is not merged to master and does not have access to pipeline secrets (for Docker login)

Only testing ipam/dropgz against swifte2e and ciliume2e scenarios. Is there anything else we want to include/exclude in the pipeline? @rbtr

Comment on lines 1 to 25
pr:
branches:
include:
- master
paths:
exclude:
- ".*"
- docs

trigger:
paths:
include:
- "zapai/*"
- "azure-ipam/*"
- "dropgz/*"
exclude:
- docs
tags:
include:
- "zapai/*"
- "azure-ipam/*"
- "dropgz/*"
Copy link
Collaborator

Choose a reason for hiding this comment

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

for includes to work as intended here, do we need to exclude everything else?

Suggested change
pr:
branches:
include:
- master
paths:
exclude:
- ".*"
- docs
trigger:
paths:
include:
- "zapai/*"
- "azure-ipam/*"
- "dropgz/*"
exclude:
- docs
tags:
include:
- "zapai/*"
- "azure-ipam/*"
- "dropgz/*"
pr:
branches:
include:
- master
paths:
exclude:
- "*"
include:
- "zapai/*"
- "azure-ipam/*"
- "dropgz/*"
trigger:
paths:
exclude:
- "*"
include:
- "zapai/*"
- "azure-ipam/*"
- "dropgz/*"
tags:
exclude:
- "*"
include:
- "zapai/*"
- "azure-ipam/*"
- "dropgz/*"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, this would make more sense

make tools
# run test, echo exit status code to fd 3, pipe output from test to tee, which splits output to stdout and go-junit-report (which converts test output to report.xml), stdout from tee is redirected to fd 4. Take output written to fd 3 (which is the exit code of test), redirect to stdout, pipe to read from stdout then exit with that status code. Read all output from fd 4 (output from tee) and write to top stdout
{ { { {
sudo -E env "PATH=$PATH" make test-all;
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems too broad

@@ -0,0 +1,29 @@
parameters:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy/pasting like this is going to create additional maintenance overhead, and the designs will immediately begin to drift apart. We should evaluate what is actually different between the two pipelines and parameterize the templates accordingly so that they are generic enough to run in both scenarios.

name: $(name)
platforms: $(platforms)

# - template: singletenancy/ipam-dropgz/ipam-dropgz-cilium-e2e-job-template.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

I see this template commented out, should it be enabled? I couldn't find any other reference to ipam-dropgz-cilium-e2e-job or the swift one

Copy link
Contributor Author

@camrynl camrynl Oct 6, 2022

Choose a reason for hiding this comment

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

I commented it out while I was testing a new parameter with the original swifte2e and ciliume2e template. I am planning to remove the ipam-dropgz templates

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm restructuring things based off of the feedback I've gotten so far

@camrynl
Copy link
Contributor Author

camrynl commented Oct 13, 2022

estebancams
estebancams previously approved these changes Oct 13, 2022
rbtr
rbtr previously approved these changes Oct 14, 2022
@camrynl camrynl enabled auto-merge (squash) October 14, 2022 16:35
@camrynl camrynl dismissed stale reviews from rbtr and estebancams via 34da572 October 14, 2022 18:22
@camrynl camrynl requested review from estebancams and rbtr October 14, 2022 19:08
@rbtr rbtr disabled auto-merge October 17, 2022 21:13
@rbtr rbtr merged commit 96c113a into Azure:master Oct 17, 2022
rjdenney pushed a commit to rjdenney/azure-container-networking that referenced this pull request Jan 19, 2023
* add new ipam/dropgz e2e test stage

* set test image

* set paths in pipeline.yaml

* set path trigger in ipam-dropgz-job template

* set path as param for ipam-dropgze2e

* testing path trigger with branch

* create submodule pipeline

* add log in /azure-ipam to test piepline trigger

* remove build for cni-dropgz image in submodule pipeline

* create separate swift + cilium ipam/dropgz test stages

* remove commented test bits

* remove commented lines from setup_test.go

* update paths/triggers and set template parameters

* change cleanup dependsOn

* test parameter call

* export parameter

* skip setting testDropgz instead of setting false

* removing ipam-dropgz templates and focus submod UT on azure-ipam

* remove parsing gatewayIP from azure-ipam unit tests

* update make target test-azure-ipam

* update make target

* only publish test results

* omit npm build

* keep npm build and simplify unit testing stage

* adjust cluster naming to avoid resource overlap in pipelines
@camrynl camrynl deleted the testDropgz branch September 28, 2023 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Infra or tooling.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants