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.

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

Add Mariner2.0 #350

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yakovdyadkin
Copy link
Contributor

@yakovdyadkin yakovdyadkin commented Jun 3, 2024

Add customization scripts for CBL-Mariner2.0

TODO: add more detail.

@@ -63,6 +78,13 @@ then
rmmod nvidia_peermem
fi

# Mariner only
if lsmod | grep nv_peer_mem &> /dev/null
Copy link
Contributor

Choose a reason for hiding this comment

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

is this only for certain SKUs? or all SKUs?

common/setup_sku_customizations.sh Outdated Show resolved Hide resolved
@@ -12,6 +12,10 @@ find_distro() {
then
local ubuntu_distro=`find_ubuntu_distro`
echo "${os} ${ubuntu_distro}"
elif [[ $os == "Common Base Linux Mariner" ]]
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems very specific. Is $DISTRIBUTION in scope at this point? If so, can we use that rather than reparsing the OS in a different way, and use fuzzy matching?

Copy link
Contributor

@darkwhite29 darkwhite29 Jun 8, 2024

Choose a reason for hiding this comment

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

Starting from release 2.0.20240301, Common Base Linux Mariner (CBL-Mariner) is rebranded as Azure Linux. Users may not see the word "Mariner" as the OS name -- we need to consider this in our OS name matching.

In our code, we can still call it Mariner, like the function find_mariner_distro() can be kept as is (for simplicity), but we need to be careful in the OS name matching here, as the word "Mariner" may not appear in any forms in the OS.

Currently, the VM OS info shown still says Mariner:

litan2@litan2southcentralusk-mariner-mofed [ ~ ]$ cat /etc/os-release     
NAME="Common Base Linux Mariner"
VERSION="2.0.20240425"
ID=mariner
VERSION_ID="2.0"
PRETTY_NAME="CBL-Mariner/Linux"
ANSI_COLOR="1;34"
HOME_URL="https://aka.ms/cbl-mariner"
BUG_REPORT_URL="https://aka.ms/cbl-mariner"
SUPPORT_URL="https://aka.ms/cbl-mariner"

But this may change in the future, as the rebranding progresses -- we need to keep an eye on when it's updated.

As I can see from the above official homepage of Mariner (https://aka.ms/cbl-mariner), the GitHub repo name has been changed, but not yet in the README.

# Find Mariner distro
find_mariner_distro() {
echo $(cat /etc/os-release | awk 'match($0, /^VERSION_ID="(.*)"/, result) { print result[1] }')
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above. Can we use $DISTRIBUTION rather than these functions?

@@ -26,7 +26,13 @@ then
fi

## load nvidia-peermem module
modprobe nvidia-peermem
os=$(cat /etc/os-release | grep "^ID=" | awk -F"=" '{print $NF}')
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I really don't know if it's in scope for these customization files, but same question about $DISTRIBUTION

# Kernel
export KERNEL=$(rpm -q kernel | sed 's/kernel\-//g')

$COMMON_DIR/write_component_version.sh "distribution" $DISTRIBUTION
Copy link
Contributor

Choose a reason for hiding this comment

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

interesting. I think i'm doing this in the build pipeline. Do you think it's a better idea here?

tarball="MLNX_OFED_SRC-$mofed_version.tgz"
mofed_download_url=https://www.mellanox.com/downloads/ofed/MLNX_OFED-$mofed_version/$tarball
mofed_folder=$(basename $mofed_download_url .tgz)
kernel_without_arch="${KERNEL%.*}"
Copy link
Contributor

Choose a reason for hiding this comment

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

it doesn't look like this gets used anymore. At one point i think it was needed either in the download file name or a compile switch.

tdnf repolist

# Install Kernel dependencies
tdnf install -y kernel-headers-$(uname -r) \
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a case where I thought you might use $KERNEL (though i don't know if it's the same format)

# tdnf install -y pssh-$pssh_version.el8.noarch.rpm
# rm -f pssh-$pssh_version.el8.noarch.rpm

tdnf install -y python3-devel \
Copy link
Contributor

Choose a reason for hiding this comment

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

have we re-evaluated these since removing spack?

rm -rf ./packages.microsoft.com/

# Create alias for "ls -l"
echo "alias ll='ls -l'" | tee -a /etc/bash.bashrc
Copy link
Contributor

Choose a reason for hiding this comment

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

What about azcopy and the kvp client? Or are those installed elsewhere?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants