-
Notifications
You must be signed in to change notification settings - Fork 371
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
Install agent on test VMs #2714
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2714 +/- ##
===========================================
+ Coverage 71.95% 71.96% +0.01%
===========================================
Files 104 104
Lines 15765 15765
Branches 2244 2244
===========================================
+ Hits 11343 11345 +2
- Misses 3906 3909 +3
+ Partials 516 511 -5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
@@ -17,8 +17,6 @@ develop-eggs/ | |||
dist/ | |||
downloads/ | |||
eggs/ | |||
lib/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my previous PR I wanted to use "lib" as the name for directories containing Python libraries, but we were ignoring those. At the time, I thought "lib" was created by makepkg.py, but that it not the case. I do not know why this ignore rule was added in the first place, but it is not needed so I am removing it (and I'm renaming the "modules" directory to "lib").
The same applies for "lib64"
@@ -1,14 +1,15 @@ | |||
#!/usr/bin/env python | |||
#!/usr/bin/env python3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newer distros don't have a default "python". Using "python3" explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After this change, would this break on python distros if no py3?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but we don't use any of those distros in our automation or deployment pipelines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we rely on makepkg.py to install agent in new automation it will break for distros if we onboard them like rhel 7 or centos 7 distros
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we control the orchestrator VM, which is where we build the package. Currently it is Ubuntu 18, but soon Azure Pipelines will move to Ubuntu 20.
The container for the deployment pipeline is also Ubuntu 18.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see we build the egg inside container and move to the vm. Thanks for explaining
@@ -1,14 +1,15 @@ | |||
#!/usr/bin/env python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed makepkg.py so that it can be used as a script (as it has been used so far) and also as a library function. There are very few code changes, other than the introduction of the run() function and the "if name == "main":" pattern. I pointed out the actual code changes with PR comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When will we use this as a library function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's been called from AgentTestSuite._build_agent_package()
print(e.output) | ||
sys.exit(1) | ||
|
||
raise Exception("[{0}] failed:\n{1}\n{2}".format(" ".join(args), str(e), e.stdout)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Raising an exception instead of using print() and sys.exit()
raise Exception("[{0}] failed:\n{1}\n{2}".format(" ".join(args), str(e), e.stdout)) | ||
|
||
|
||
def run(agent_family, output_directory, log): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrapped the code of the function in a run() function
self.custom_script() | ||
|
||
def check_agent_version(self) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer needed, since now the agent version is checked when the agent package is installed
@@ -1,53 +0,0 @@ | |||
from pathlib import Path, PurePath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced by tests_e2e/orchestrator/lib/agent_test_suite.py
@@ -41,30 +41,30 @@ jobs: | |||
include: | |||
|
|||
- python-version: 2.7 | |||
PYLINTOPTS: "--rcfile=ci/2.7.pylintrc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the end-to-end tests to the pylint run, but only on the most recent Python we test. This code uses language features not available in older versions so it is not worth linting there.
Excluded azure_models.py and BaseExtensionTestClass.py, since they have multiple warnings. These files come from DCR and need to be rewritten anyway.
Lastly, excluded makepkg.py from the runs on 2.7 and 3.4. Some of the API calls I added do not exist on those versions.
@@ -1,18 +0,0 @@ | |||
#!/usr/bin/env bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file is not used anywhere, deleted it
@@ -1,32 +0,0 @@ | |||
#!/usr/bin/env bash | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file is not used anywhere, deleted it
@@ -44,7 +44,6 @@ variable: | |||
value: "" | |||
is_secret: true | |||
notifier: | |||
- type: html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need the html report
@@ -1,14 +1,15 @@ | |||
#!/usr/bin/env python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When will we use this as a library function?
@@ -0,0 +1,105 @@ | |||
#!/usr/bin/env bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will we be able to use this script across all distros?
@@ -1,14 +1,15 @@ | |||
#!/usr/bin/env python | |||
#!/usr/bin/env python3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After this change, would this break on python distros if no py3?
# | ||
echo "Installing $package..." | ||
unzip -d "/var/lib/waagent/WALinuxAgent-$version" -o "$package" | ||
sed -i 's/AutoUpdate.Enabled=n/AutoUpdate.Enabled=y/g' /etc/waagent.conf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this autoupdate? I think we should pin the test version to run our scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no way to pin the version of the self-updated agent, so we deploy a new package as 9.9.9.9 and ensure auto-update is enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even we ensure auto update is enabled its no-op on agent update in test package 9.9.9.9 version except downloading extra pkgs in the vm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I don't get your comment. If auto-update is disabled the daemon won't start 9.9.9.9 (or any of the other self-update versions).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, I keep forgetting that check in daemon. we are good then
@@ -0,0 +1,19 @@ | |||
#!/usr/bin/env bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why these scripts moved here? These are executed by lisa per scenario. Isn't scenarios best place to keep? same thing for agent_test_suite.
print("Creating egg {0}".format(egg_path)) | ||
do(*args) | ||
if __name__ == "__main__": | ||
logging.basicConfig(format='%(message)s', level=logging.INFO) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where are the log msgs stored? which file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the py file is invoked as a script (name == "main") we just to output to stdout/stderr , no file is created. Same behavior as the previous version of this script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the other hand, when the py file is used as a library module, the caller passes the log to the run() method.
In our case, the test suite is passing LISA's log, so during test runs the output goes to the test log file (on top of stdout/stderr).
@@ -1,14 +1,15 @@ | |||
#!/usr/bin/env python | |||
#!/usr/bin/env python3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see we build the egg inside container and move to the vm. Thanks for explaining
Now we install the test agent on the test VMs before executing the tests.
The PR also includes some re-structuring of the code and some minor cleanup; I pointed out those changes with comments in the PR.