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.

Sign up for GitHub

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

Jump to bottom

Install agent on test VMs #2714

Merged
merged 14 commits into from
Dec 15, 2022
Merged

Install agent on test VMs #2714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
f756604
Install agent on test VMs
Dec 13, 2022
035bdca
install distro module
Dec 13, 2022
b2a81a2
capture command error message
Dec 13, 2022
a0df4ef
.
Dec 13, 2022
f733474
.
Dec 13, 2022
00cb310
.
Dec 13, 2022
29425ef
.
Dec 13, 2022
27ac4b0
.
Dec 13, 2022
653ed34
.
Dec 14, 2022
eacbec5
.
Dec 14, 2022
d2710f0
.
Dec 14, 2022
251a372
.
Dec 14, 2022
b6ad492
.
Dec 14, 2022
76c2d21
.
Dec 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions .github/workflows/ci_pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,30 +41,30 @@ jobs:
include:

- python-version: 2.7
PYLINTOPTS: "--rcfile=ci/2.7.pylintrc"
Copy link
Member Author

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.

PYLINTOPTS: "--rcfile=ci/2.7.pylintrc --ignore=tests_e2e,makepkg.py"

- python-version: 3.4
PYLINTOPTS: "--rcfile=ci/2.7.pylintrc"
PYLINTOPTS: "--rcfile=ci/2.7.pylintrc --ignore=tests_e2e,makepkg.py"

- python-version: 3.6
PYLINTOPTS: "--rcfile=ci/3.6.pylintrc"
PYLINTOPTS: "--rcfile=ci/3.6.pylintrc --ignore=tests_e2e"

- python-version: 3.7
PYLINTOPTS: "--rcfile=ci/3.6.pylintrc"
PYLINTOPTS: "--rcfile=ci/3.6.pylintrc --ignore=tests_e2e"

- python-version: 3.8
PYLINTOPTS: "--rcfile=ci/3.6.pylintrc"
PYLINTOPTS: "--rcfile=ci/3.6.pylintrc --ignore=tests_e2e"

- python-version: 3.9
PYLINTOPTS: "--rcfile=ci/3.6.pylintrc"
PYLINTOPTS: "--rcfile=ci/3.6.pylintrc --ignore=azure_models.py,BaseExtensionTestClass.py"
additional-nose-opts: "--with-coverage --cover-erase --cover-inclusive --cover-branches --cover-package=azurelinuxagent"

name: "Python ${{ matrix.python-version }} Unit Tests"
runs-on: ubuntu-18.04

env:
PYLINTOPTS: ${{ matrix.PYLINTOPTS }}
PYLINTFILES: "azurelinuxagent setup.py makepkg.py tests"
PYLINTFILES: "azurelinuxagent setup.py makepkg.py tests tests_e2e"
NOSEOPTS: "--with-timer ${{ matrix.additional-nose-opts }}"
PYTHON_VERSION: ${{ matrix.python-version }}

Expand Down
2 changes: 0 additions & 2 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ develop-eggs/
dist/
downloads/
eggs/
lib/
Copy link
Member Author

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"

lib64/
parts/
sdist/
var/
Expand Down
18 changes: 0 additions & 18 deletions ci/nosetests_only.sh
View file
Open in desktop

This file was deleted.

32 changes: 0 additions & 32 deletions ci/pylint_and_nosetests.sh
View file
Open in desktop

This file was deleted.

118 changes: 67 additions & 51 deletions makepkg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
#!/usr/bin/env python
Copy link
Member Author

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.

Copy link
Contributor

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?

Copy link
Member Author

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()

#!/usr/bin/env python3
Copy link
Member Author

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.

Copy link
Contributor

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?

Copy link
Member Author

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.

Copy link
Contributor

@nagworld9 nagworld9 Dec 15, 2022 •
edited
Loading

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

Copy link
Member Author

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.

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 we build the egg inside container and move to the vm. Thanks for explaining


import argparse
import glob
import os
import logging
import os.path
import pathlib
import shutil
import subprocess
import sys

from azurelinuxagent.common.version import AGENT_NAME, AGENT_VERSION, \
AGENT_LONG_VERSION
from azurelinuxagent.common.version import AGENT_NAME, AGENT_VERSION, AGENT_LONG_VERSION
from azurelinuxagent.ga.update import AGENT_MANIFEST_FILE

MANIFEST = '''[{{
Expand Down Expand Up @@ -48,62 +49,77 @@

PUBLISH_MANIFEST_FILE = 'manifest.xml'

output_path = os.path.join(os.getcwd(), "eggs") # pylint: disable=invalid-name
target_path = os.path.join(output_path, AGENT_LONG_VERSION) # pylint: disable=invalid-name
bin_path = os.path.join(target_path, "bin") # pylint: disable=invalid-name
egg_path = os.path.join(bin_path, AGENT_LONG_VERSION + ".egg") # pylint: disable=invalid-name
manifest_path = os.path.join(target_path, AGENT_MANIFEST_FILE) # pylint: disable=invalid-name
publish_manifest_path = os.path.join(target_path, PUBLISH_MANIFEST_FILE) # pylint: disable=invalid-name
pkg_name = os.path.join(output_path, AGENT_LONG_VERSION + ".zip") # pylint: disable=invalid-name

family = 'Test' # pylint: disable=C0103
if len(sys.argv) > 1:
family = sys.argv[1] # pylint: disable=invalid-name

def do(*args): # pylint: disable=C0103,W0621
def do(*args):
try:
subprocess.check_output(args, stderr=subprocess.STDOUT)
return subprocess.check_output(args, stderr=subprocess.STDOUT)
except subprocess.CalledProcessError as e: # pylint: disable=C0103
print("ERROR: {0}".format(str(e)))
print("\t{0}".format(" ".join(args)))
print(e.output)
sys.exit(1)

raise Exception("[{0}] failed:\n{1}\n{2}".format(" ".join(args), str(e), e.stdout))
Copy link
Member Author

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()



def run(agent_family, output_directory, log):
Copy link
Member Author

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

output_path = os.path.join(output_directory, "eggs")
target_path = os.path.join(output_path, AGENT_LONG_VERSION)
bin_path = os.path.join(target_path, "bin")
egg_path = os.path.join(bin_path, AGENT_LONG_VERSION + ".egg")
manifest_path = os.path.join(target_path, AGENT_MANIFEST_FILE)
publish_manifest_path = os.path.join(target_path, PUBLISH_MANIFEST_FILE)
pkg_name = os.path.join(output_path, AGENT_LONG_VERSION + ".zip")

if os.path.isdir(target_path):
shutil.rmtree(target_path)
elif os.path.isfile(target_path):
os.remove(target_path)
if os.path.isfile(pkg_name):
os.remove(pkg_name)
os.makedirs(bin_path)
log.info("Created {0} directory".format(target_path))
Copy link
Member Author

Choose a reason for hiding this comment

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

using logging.info() instead of print()


setup_script = str(pathlib.Path(__file__).parent.joinpath("setup.py"))
args = ["python3", setup_script, "bdist_egg", "--dist-dir={0}".format(bin_path)]
Copy link
Member Author

Choose a reason for hiding this comment

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

using "python3" instead of "python"


log.info("Creating egg {0}".format(egg_path))
do(*args)

egg_name = os.path.join("bin", os.path.basename(
glob.glob(os.path.join(bin_path, "*"))[0]))

log.info("Writing {0}".format(manifest_path))
with open(manifest_path, mode='w') as manifest:
manifest.write(MANIFEST.format(AGENT_NAME, egg_name))

log.info("Writing {0}".format(publish_manifest_path))
with open(publish_manifest_path, mode='w') as publish_manifest:
publish_manifest.write(PUBLISH_MANIFEST.format(AGENT_VERSION, agent_family))

cwd = os.getcwd()
os.chdir(target_path)
try:
Copy link
Member Author

Choose a reason for hiding this comment

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

since now this can be used as a library function, added try/finally to cover the calls to os.chdir()

log.info("Creating package {0}".format(pkg_name))
do("zip", "-r", pkg_name, egg_name)
do("zip", "-j", pkg_name, AGENT_MANIFEST_FILE)
do("zip", "-j", pkg_name, PUBLISH_MANIFEST_FILE)
finally:
os.chdir(cwd)

if os.path.isdir(target_path):
shutil.rmtree(target_path)
elif os.path.isfile(target_path):
os.remove(target_path)
if os.path.isfile(pkg_name):
os.remove(pkg_name)
os.makedirs(bin_path)
print("Created {0} directory".format(target_path))
log.info("Package {0} successfully created".format(pkg_name))

args = ["python", "setup.py", "bdist_egg", "--dist-dir={0}".format(bin_path)] # pylint: disable=invalid-name

print("Creating egg {0}".format(egg_path))
do(*args)
if __name__ == "__main__":
Copy link
Member Author

Choose a reason for hiding this comment

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

Added the "if name == "main":" so that this can be used as a script or as a library function

logging.basicConfig(format='%(message)s', level=logging.INFO)
Copy link
Contributor

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?

Copy link
Member Author

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.

Copy link
Member Author

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).


egg_name = os.path.join("bin", os.path.basename( # pylint: disable=invalid-name
glob.glob(os.path.join(bin_path, "*"))[0]))
parser = argparse.ArgumentParser()
parser.add_argument('family', metavar='family', nargs='?', default='Test', help='Agent family')
parser.add_argument('-o', '--output', default=os.getcwd(), help='Output directory')

print("Writing {0}".format(manifest_path))
with open(manifest_path, mode='w') as manifest:
manifest.write(MANIFEST.format(AGENT_NAME, egg_name))
arguments = parser.parse_args()

print("Writing {0}".format(publish_manifest_path))
with open(publish_manifest_path, mode='w') as publish_manifest:
publish_manifest.write(PUBLISH_MANIFEST.format(AGENT_VERSION,
family))
try:

run(arguments.family, arguments.output, logging)

cwd = os.getcwd() # pylint: disable=invalid-name
os.chdir(target_path)
print("Creating package {0}".format(pkg_name))
do("zip", "-r", pkg_name, egg_name)
do("zip", "-j", pkg_name, AGENT_MANIFEST_FILE)
do("zip", "-j", pkg_name, PUBLISH_MANIFEST_FILE)
os.chdir(cwd)
except Exception as exception:
logging.error(str(exception))
sys.exit(1)

print("Package {0} successfully created".format(pkg_name))
sys.exit(0)
sys.exit(0)
5 changes: 4 additions & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,10 @@ def initialize_options(self):
self.lnx_distro_version = DISTRO_VERSION
self.lnx_distro_fullname = DISTRO_FULL_NAME
self.register_service = False
self.skip_data_files = False
# All our data files are system-wide files that are not included in the egg; skip them when
Copy link
Member Author

Choose a reason for hiding this comment

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

It has always bothered me that one needs to execute makepkg.py with sudo. The reason is that setup.py (called by makepkg.py) is installing these data files, which in our case are all system files in locations as /etc, /usr/lib, etc. Those files are not added to the egg, so they can be skipped when using "bdist_egg" (as makepkg.py does).

Now makepkg.py does not require to be invoked with sudo.

# creating an egg.
self.skip_data_files = "bdist_egg" in sys.argv

# pylint: enable=attribute-defined-outside-init

def finalize_options(self):
Expand Down
2 changes: 1 addition & 1 deletion tests_e2e/docker/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ RUN \
# \
# Install additional test dependencies \
# \
python3 -m pip install msrestazure && \
python3 -m pip install distro msrestazure && \
Copy link
Member Author

Choose a reason for hiding this comment

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

Added "distro" because now we are importing "version.py" (which has a dependency on "distro") to get the version of the test agent.

\
# \
# The setup for the tests depends on a couple of paths; add those to the profile \
Expand Down
Loading