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

Add mariner 1.0 to DCR v2 #2517

Merged
merged 5 commits into from
Feb 24, 2022
Merged

Add mariner 1.0 to DCR v2 #2517

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
58d385d
Add fixes for compatibility with mariner
Feb 10, 2022
79cd555
Update explicit disk size to shared minimum
Feb 10, 2022
2b00a1d
Merge branch 'develop' of https://github.com/Azure/WALinuxAgent into …
Feb 22, 2022
49d4133
Change chmod opening permissions to a chown.
Feb 23, 2022
9eb4e01
updated comment to better reflect command.
Feb 23, 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
6 changes: 6 additions & 0 deletions dcr/azure-pipelines.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ parameters:
sku: "7-RAW"
version: "latest"
name: "rhel7Raw"
##
- publisher: "microsoftcblmariner"
offer: "cbl-mariner"
sku: "cbl-mariner-1"
version: "latest"
name: "mariner1"

trigger:
- develop
Expand Down
4 changes: 2 additions & 2 deletions dcr/scenarios/agent-bvt/check_firewall.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ def __is_firewall_enabled():
if update_match:
return update_match.groups()[0].lower() == 'y'

# The firewall is enabled by default.
return True
# The firewall is disabled by default.
return False
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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



def run(*args):
Expand Down
2 changes: 1 addition & 1 deletion dcr/scenarios/agent-bvt/test_agent_basics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Member

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 test at all? what is its equivalent in DCR v1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, will do

return 'root login disabled'
else:
raise Exception('root login appears to be enabled: {0}'.format(root_passwd))
Expand Down
3 changes: 3 additions & 0 deletions dcr/scripts/test-vm/harvest.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
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), so change the
# owning user so we can grab them below (during the scp command).
ssh -o "StrictHostKeyChecking no" "$1"@"$2" "sudo chown $1 logs-$2.tgz"
Copy link
Member

Choose a reason for hiding this comment

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

comment on previous line needs updating

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch!


# Create directory if doesn't exist
mkdir -p "$3"
Expand Down
3 changes: 2 additions & 1 deletion dcr/templates/deploy-linux-vm.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,8 @@
"createOption": "FromImage",
"managedDisk": {
"storageAccountType": "[variables('osDiskType')]"
}
},
"diskSizeGB": 32
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

},
"imageReference": {
"publisher": "[parameters('imagePublisher')]",
Expand Down