Add mariner 1.0 to DCR v2 #2517

kevinclark19a · 2022-02-22T17:54:42Z

Description

This PR adds end-to-end testing for the mariner 1.0 image on the marketplace.

PR information

The title of the PR is clear and informative.
There are a small number of commits, each of which has an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
If applicable, the PR references the bug/issue that it fixes in the description.
New Unit tests were added for the changes made

Quality of Code and Contribution Guidelines

I have read the contribution guidelines.

…add_mariner_dcr_v2

kevinclark19a · 2022-02-22T17:55:31Z

dcr/scenarios/agent-bvt/check_firewall.py

-    # The firewall is enabled by default.
-    return True
+    # The firewall is disabled by default.
+    return False


As seen with mariner 1.0, when there is no entry in the config file, the agent disables the firewall.

kevinclark19a · 2022-02-22T17:58:20Z

dcr/scenarios/agent-bvt/test_agent_basics.py

@@ -48,7 +48,7 @@ def check_root_login():
    print(root_passwd_line)
    root_passwd = root_passwd_line.split(":")[1]

-    if "!" in root_passwd or "*" in root_passwd:
+    if any(val in root_passwd for val in ("!", "*", "x")):


Mariner 1.0 uses a different syntax to mean the same thing; x is not the result of a hash and therefore root password is disabled. Maybe we want to have a less explicit check here, so that we don't need to keep updating this code? Something to the effect of checking the length of the entry might work (i.e. anything a single character long is not a hash), but I'm not sure. Thoughts?

why do we need this test at all? what is its equivalent in DCR v1?

The equivalent is: https://msazure.visualstudio.com/DefaultCollection/One/_git/Compute-Runtime-Tux-Pipeline?path=%2Fdungeon_crawler%2Fscenarios%2Fagent-bvt%2F05_check_root_login.py&_a=contents&version=GBdevelop

My first impression is that we can drop this test. Before doing so, though, could you check if the agent disables root login (maybe during provisioning)?

Sure, will do

codecov · 2022-02-22T17:58:24Z

Codecov Report

Merging #2517 (9eb4e01) into develop (899a38e) will increase coverage by 0.03%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #2517      +/-   ##
===========================================
+ Coverage    71.91%   71.94%   +0.03%     
===========================================
  Files          101      101              
  Lines        15147    15147              
  Branches      2401     2401              
===========================================
+ Hits         10893    10898       +5     
+ Misses        3773     3771       -2     
+ Partials       481      478       -3

Impacted Files	Coverage Δ
azurelinuxagent/ga/collect_telemetry_events.py	`91.43% <0.00%> (+1.71%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 899a38e...9eb4e01. Read the comment docs.

kevinclark19a · 2022-02-22T18:00:59Z

dcr/templates/deploy-linux-vm.json

@@ -234,7 +234,8 @@
            "createOption": "FromImage",
            "managedDisk": {
              "storageAccountType": "[variables('osDiskType')]"
-            }
+            },
+            "diskSizeGB": 32


Mariner 1.0 specifies a minimum OS disk as 1GB, which causes our testing to crash with "no space left on disk" if we leave this property blank. This approach does lock in all other distros to the same size; i.e., I believe ubuntu lets you create an 16GB OS disk, so we'd be doubling that size with this. 32GB is the largest minimum of the distros we test, so I went with that.

narrieta · 2022-02-23T00:30:04Z

dcr/scripts/test-vm/harvest.sh

@@ -11,6 +11,8 @@
 set -euxo pipefail

 ssh -o "StrictHostKeyChecking no" "$1"@"$2" "sudo tar --exclude='journal/*' --exclude='omsbundle' --exclude='omsagent' --exclude='mdsd' --exclude='scx*' --exclude='*.so' --exclude='*__LinuxDiagnostic__*' --exclude='*.zip' --exclude='*.deb' --exclude='*.rpm' -czf logs-$2.tgz /var/log /var/lib/waagent/ /etc/waagent.conf"
+# Some distros do not have "other" permissions (e.g., mariner1.0), add them here.
+ssh -o "StrictHostKeyChecking no" "$1"@"$2" "sudo chmod o+rwx logs-$2.tgz"


why do we need the "other" permission?

The harvest scripts fail without them on mariner; I assume we're using a different user for harvest, but I'm not sure; I'll check and update here.

we are creating the file using sudo, so the owner would be root... what user is harvest running under? "edp"?

if that is the case, i would just update the comment on line 14 to mention that (or, maybe better, chown the file)

Yep, I think you're right; the difference is that mariner defaults new files to more restrictive permissions than other distros. (The call to copy the tar over is the last line of this file).

The equivalent of adding sudo to the harvest command would be using root as the user instead of dcr in the scp command. I think using the root user directly would be less ideal than this chmod, though.

ah, ok, then $1 is the user... yes, probably the chown would be slightly better than a blanket all permissions to all users

narrieta · 2022-02-23T00:32:05Z

@kevinclark19a can you post a link to a sample run?

kevinclark19a · 2022-02-23T19:28:51Z

@narrieta, here's the sample run. It has the ETP failures that are on develop.

narrieta · 2022-02-23T19:48:06Z

@kevinclark19a run looks good, thanks

narrieta · 2022-02-23T21:51:05Z

dcr/scripts/test-vm/harvest.sh

@@ -12,7 +12,7 @@ set -euxo pipefail

 ssh -o "StrictHostKeyChecking no" "$1"@"$2" "sudo tar --exclude='journal/*' --exclude='omsbundle' --exclude='omsagent' --exclude='mdsd' --exclude='scx*' --exclude='*.so' --exclude='*__LinuxDiagnostic__*' --exclude='*.zip' --exclude='*.deb' --exclude='*.rpm' -czf logs-$2.tgz /var/log /var/lib/waagent/ /etc/waagent.conf"
 # Some distros do not have "other" permissions (e.g., mariner1.0), add them here.
-ssh -o "StrictHostKeyChecking no" "$1"@"$2" "sudo chmod o+rwx logs-$2.tgz"
+ssh -o "StrictHostKeyChecking no" "$1"@"$2" "sudo chown $1 logs-$2.tgz"


comment on previous line needs updating

good catch!

Kevin added 3 commits February 10, 2022 19:00

Add fixes for compatibility with mariner

58d385d

Update explicit disk size to shared minimum

79cd555

Merge branch 'develop' of https://github.com/Azure/WALinuxAgent into …

2b00a1d

…add_mariner_dcr_v2

kevinclark19a requested review from narrieta, larohra, ZhidongPeng, dhivyaganesan and nagworld9 as code owners February 22, 2022 17:54

kevinclark19a commented Feb 22, 2022

View reviewed changes

narrieta reviewed Feb 23, 2022

View reviewed changes

kevinclark19a assigned larohra and nagworld9 Feb 23, 2022

nagworld9 previously approved these changes Feb 23, 2022

View reviewed changes

Change chmod opening permissions to a chown.

49d4133

kevinclark19a dismissed nagworld9’s stale review via 49d4133 February 23, 2022 21:32

narrieta reviewed Feb 23, 2022

View reviewed changes

updated comment to better reflect command.

9eb4e01

narrieta approved these changes Feb 23, 2022

View reviewed changes

nagworld9 approved these changes Feb 23, 2022

View reviewed changes

kevinclark19a merged commit 3036ae5 into Azure:develop Feb 24, 2022

kevinclark19a deleted the add_mariner_dcr_v2 branch March 9, 2022 23:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add mariner 1.0 to DCR v2 #2517

Add mariner 1.0 to DCR v2 #2517

kevinclark19a commented Feb 22, 2022

kevinclark19a Feb 22, 2022

kevinclark19a Feb 22, 2022

narrieta Feb 23, 2022

kevinclark19a Feb 23, 2022

narrieta Feb 23, 2022

kevinclark19a Feb 23, 2022

codecov bot commented Feb 22, 2022 •

edited

Loading

kevinclark19a Feb 22, 2022

narrieta Feb 23, 2022 •

edited

Loading

kevinclark19a Feb 23, 2022

narrieta Feb 23, 2022 •

edited

Loading

kevinclark19a Feb 23, 2022

narrieta Feb 23, 2022

narrieta commented Feb 23, 2022

kevinclark19a commented Feb 23, 2022

narrieta commented Feb 23, 2022

narrieta Feb 23, 2022

kevinclark19a Feb 23, 2022

Add mariner 1.0 to DCR v2 #2517

Add mariner 1.0 to DCR v2 #2517

Conversation

kevinclark19a commented Feb 22, 2022

Description

PR information

Quality of Code and Contribution Guidelines

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Feb 22, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

narrieta Feb 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

narrieta Feb 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

narrieta commented Feb 23, 2022

kevinclark19a commented Feb 23, 2022

narrieta commented Feb 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Feb 22, 2022 •

edited

Loading

narrieta Feb 23, 2022 •

edited

Loading

narrieta Feb 23, 2022 •

edited

Loading