Improvements in Pipeline parameters #2748
Conversation
narrieta
requested review from
ZhidongPeng,
nagworld9 and
maddieford
as code owners
| @@ -73,5 +73,6 @@ RUN \ | |||
| # \ | |||
| echo 'export PYTHONPATH="$HOME/WALinuxAgent"' >> $HOME/.bash_profile && \ | |||
| echo 'export PATH="$HOME/.local/bin:$PATH"' >> $HOME/.bash_profile && \ | |||
| echo 'cd $HOME' >> $HOME/.bash_profile && \ | |||
There was a problem hiding this comment.
Every time I start an interactive container, I need to cd to home. Adding it here for convenience.
There was a problem hiding this comment.
(container image in the repo is updated)
| - name: subscription_id | ||
| value: "" | ||
| - name: keep_environment | ||
| value: "no" | ||
| - name: wait_delete |
There was a problem hiding this comment.
Remove this runbook parameter. By default the runbook starts deleting the test VMs asynchronously and does not wait for the deletion to complete. This parameter can force a wait on the delete operation, but it is not needed by the automation pipeline.
I moved keep_environment to the end of the list of runbook parameters
| @@ -48,25 +66,6 @@ variable: | |||
| - name: default_location | |||
| value: "westus2" | |||
|
|
|||
| # | |||
There was a problem hiding this comment.
Moved this block before the SSH parameters
| @@ -1,83 +0,0 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
I merge this script into execute_tests.sh
Codecov Report
@@ Coverage Diff @@
## develop #2748 +/- ##
========================================
Coverage 72.04% 72.04%
========================================
Files 104 104
Lines 15832 15832
Branches 2265 2265
========================================
Hits 11406 11406
Misses 3912 3912
Partials 514 514 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| @@ -0,0 +1,56 @@ | |||
| # | |||
There was a problem hiding this comment.
I took this from the DCR V2 prototype.
The Azure CLI task needs improvements. Currently it always succeeds. The implementation needs to distinguish between the command pipeline failing because there are no items to delete (and then the task should succeed) and when it fails for other reasons (and then the task should fail, to alert us of those issues).
So far, the cleanup logic in LISA has been pretty effective, but since I added the keep_environment parameter to the main pipeline, I decided to bring along this pipeline to clean left over machines.
There was a problem hiding this comment.
where are you triggering this pipeline? also, I don't see keep_environment check to call this pipeline in this PR.
There was a problem hiding this comment.
I stopped the DCR V2 Cleanup pipeline (no point in running it, since DCR V2 is disabled). Once this PR is merged to develop, I'll point that pipeline to this yml file and start it again.
Keep environment is not related to this cleanup pipeline. Is it a parameter for the main pipeline (the pipeline that executes tests). If somebody uses Keep environment and does not cleanup all the machines, then this cleanup will do it after a day.
There was a problem hiding this comment.
oh, I was thinking like we have it for DCR. we hold it for brief period and when hit continue cleanup it would trigger cleanup pipeline to remove immediately instead of waiting for daily pipeline.
I feel like we need that ability to cleanup when we ready to destroy those otherwise at one point we end of hanging too many resources and waiting for daily pipeline to cleanup those
There was a problem hiding this comment.
I may implement that at some point, but it has very low priority. On DCR it is needed because the only way to run the tests is via Jenkins. With the new pipeline one can run the tests from our dev machines. Just set a breakpoint after all the tests have run, but before we exit AgentTestSuite.execute(), do your debugging and when you are done hit 'go' and let LISA do the cleanup.
| mkdir ssh | ||
| cp "$DOWNLOADSSHKEY_SECUREFILEPATH" ssh | ||
| chmod 700 ssh/id_rsa | ||
| ssh-keygen -y -f ssh/id_rsa > ssh/id_rsa.pub |
There was a problem hiding this comment.
this piece of code to setup the SSH key comes from the previous run-scenarios script, which I am deleting in this PR
| --env AZURE_TENANT_ID \ | ||
| waagenttests.azurecr.io/waagenttests \ | ||
| bash --login -c \ | ||
| "\$HOME/WALinuxAgent/tests_e2e/orchestrator/scripts/run-scenarios -t $TEST_SUITES -l $COLLECT_LOGS -k $SKIP_SETUP" \ |
There was a problem hiding this comment.
The TEST_SUITES parameter needs to be surrounded by quotes, in order to allow spaces in its value.
Previously we were invoking the run-scenarios script, which was basically a pass thru to LISA. With the need to pass the pipeline parameters to the LISA runbook, that script was getting over-complicated. It is just simpler to call LISA here. I did that, and removed run-scenarios.
| @@ -22,9 +21,28 @@ variable: | |||
| - name: admin_password | |||
| value: "" | |||
| is_secret: true | |||
| - name: keep_environment | |||
| value: "no" | |||
There was a problem hiding this comment.
Is this just a default value? Will this get replaced by the value for keep_environment from the azure pipeline? I dont see where we update the value
There was a problem hiding this comment.
yes, this is only the default value, good question.
these parameters get overwritten with the values from the pipeline when we invoke lisa from execute_tests.sh
"lisa \
--runbook \$HOME/WALinuxAgent/tests_e2e/orchestrator/runbook.yml \
--log_path \$HOME/logs/lisa \
--working_path \$HOME/logs/lisa \
-v subscription_id:$SUBSCRIPTION_ID \
-v identity_file:\$HOME/.ssh/id_rsa \
-v test_suites:\"$TEST_SUITES\" \
-v collect_logs:\"$COLLECT_LOGS\" \
-v keep_environment:\"$KEEP_ENVIRONMENT\**"" \
Allow spaces in the values for the pipeline parameters. This is needed in particular for test_suites, which is a comma-separated list of names and would usually include spaces between each item.
Remove parameter skip_setup. Although this parameter is useful for development scenarios and is exposed by the runbook, it really doesn't make sense as an input to the automation pipeline.
Add parameter keep_environment to skip deletion of the test VMs
Add a cleanup pipeline, which will remove any test VMs leftover from previous runs.