Skip to content

Conversation

@hanwen-pcluste
Copy link
Contributor

@hanwen-pcluste hanwen-pcluste commented Jan 16, 2024

Description of changes

  • See commits descriptions for details
  • Please ignore the last two [temporary] commits

Tests

References

Checklist

  • Make sure you are pointing to the right branch.
  • If you're creating a patch for a branch other than develop add the branch name as prefix in the PR title (e.g. [release-3.6]).
  • Check all commits' messages are clear, describing what and why vs how.
  • Make sure to have added unit tests or integration tests to cover the new/modified code.
  • Check if documentation is impacted by this change.

Please review the guidelines for contributing and Pull Request Instructions.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov
Copy link

codecov bot commented Jan 16, 2024

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (e4fee1f) 76.37% compared to head (e458114) 76.48%.

Files Patch % Lines
...nt/files/default/ec2_udev_rules/ec2_dev_2_volid.py 14.28% 6 Missing ⚠️
...nment/files/default/ec2_udev_rules/manageVolume.py 80.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2617      +/-   ##
===========================================
+ Coverage    76.37%   76.48%   +0.10%     
===========================================
  Files           22       22              
  Lines         2248     2220      -28     
===========================================
- Hits          1717     1698      -19     
+ Misses         531      522       -9     
Flag Coverage Δ
unittests 76.48% <52.94%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hanwen-pcluste hanwen-pcluste changed the title [Draft][WIP] Support for RHEL9 and Rocky9 [Develop][WIP] Support for RHEL9 and Rocky9 Jan 25, 2024
@hanwen-pcluste hanwen-pcluste marked this pull request as ready for review January 25, 2024 15:20
@hanwen-pcluste hanwen-pcluste requested review from a team as code owners January 25, 2024 15:20
@hanwen-pcluste hanwen-pcluste force-pushed the developforpr branch 4 times, most recently from 59e0a5b to d0e1afd Compare January 25, 2024 17:26
supports restart: true
action %i(enable start)
execute "Refresh UdevAdmin" do
command "udevadm trigger --action=change --subsystem-match=block"
Copy link
Contributor

Choose a reason for hiding this comment

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

May you please add a brief comment about why this is required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not 100% sure that this is required. This was done in the old code. Therefore I kept here to stay on the safe side

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's validate in a follow up activity if it is required or not. Better to not have not required steps

@@ -0,0 +1,10 @@
[Unit]
Description=Setup ephemeral drives service
Copy link
Contributor

Choose a reason for hiding this comment

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

This service is not about setting up ephemeral drivers, but about configuring iptables.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

The initial tedious changes to add RHEL9 and Rock9 to Chef resources and kitchen tests

Signed-off-by: Hanwen <hanwenli@amazon.com>
Remove `pyparted`, `redhat-lsb`, `python2`, `python2-pip` installations because they are not available in RHEL9 and Rocky9.

Add perl, dbus-devel installation because they were part of `redhat-lsb` and are dependencies of Slurm

We are doing the same on RHEL8, RHEL9, Rocky8, Rocky9 to keep consistencies across OSes.

Reference:
pyparted: https://github.com/dcantrell/pyparted
redhat-lsb: https://access.redhat.com/solutions/6973382
perl: https://www.perl.org
dbus-devel: https://packages.fedoraproject.org/pkgs/dbus/dbus-devel/

Signed-off-by: Hanwen <hanwenli@amazon.com>
ParallelCluster uses udev to trigger a Python script upon EBS attachment. Specifically, when it detects an attach from a device (e.g. /dev/xvdb), ParallelCluster udev rule creates a symbolic link from `/dev/disk/by-ebs-volumeid` (e.g. `/dev/disk/by-ebs-volumeid/vol-123456`) to the device name (/dev/xvdb). Then, our cookbook checks the device under `/dev/disk/by-ebs-volumeid` is ready.

Prior to this commit, ParallelCluster udev script used boto3 calls to retrieve volume id from device name.

Seems starting from RHEL 9, the scripts triggered by udev no long have network access for security reasons. Therefore, this commit removes the boto3 calls and get the volume id from `/dev/disk/by-ebs-volumeid/parallelcluster_dev_id_mapping`, which is a file ParallelCluster cookbook writes to. Although the logic could be further simplified by not using the udev rule at all, this commit takes the first step of improvement without spending too much time to reach the absolute simplicity.

Moreover, prior to this commit, a service was created to trigger `udevadm trigger`. `udevadm trigger` is used to refresh udev system after udev rules changes. It is unnecessary to have a service trigger the command. Therefore, this commit deletes the service and triggers the command once after the rules changes.

Signed-off-by: Hanwen <hanwenli@amazon.com>
The Munge installation is already based on systemd. The SysV Init logic is no longer used. We forgot to remove this logic when we dropped support for Amazon Linux 1

When running kitchen tests on Docker, munge on some OSes still uses SysV. Therefore, we skip some `service` resource execution. We take the kitchen tests on EC2 instance as the golden standard. This is consistent with what we've been doing (i.e. We have been skip some `service` resource for other packages)

Signed-off-by: Hanwen <hanwenli@amazon.com>
@hanwen-pcluste hanwen-pcluste force-pushed the developforpr branch 2 times, most recently from 18620fc to c797214 Compare February 13, 2024 14:25
hanwen-cluster and others added 4 commits February 13, 2024 06:45
`test_create_imds_secured` integration test has been passed

Signed-off-by: Hanwen <hanwenli@amazon.com>
Different GCC versions are used for different operating systems. For existing operating systems, we keep the version unchanged (9.3 for alinux2, centos7, ubuntu20, rhel8, rocky8; 11.3 for ubuntu22). For newly added OSes (RHEL9 and Rocky9), we use 11.3 because 9.3 is not available.

Signed-off-by: Hanwen <hanwenli@amazon.com>
For other OSes, this commit keeps the old approach of changing grub files. This is done because the old approach does not take effect on RHEL9 and Rocky9. The `grubby` command does not take effect on some of the old OSes.

Signed-off-by: Hanwen <hanwenli@amazon.com>
1. /etc/init.d is no long used. Therefore, the mocks are removed
2. Install Python. RHEL9 docker does not come with Python. We ere just lucky that previous docker had Python installed

Signed-off-by: Hanwen <hanwenli@amazon.com>
@hanwen-pcluste hanwen-pcluste changed the title [Develop][WIP] Support for RHEL9 and Rocky9 [Develop] Support for RHEL9 and Rocky9 Feb 13, 2024
@hanwen-pcluste hanwen-pcluste enabled auto-merge (rebase) February 14, 2024 14:40
@hanwen-pcluste hanwen-pcluste merged commit 3c75a58 into aws:develop Feb 14, 2024
hanwen-pcluste pushed a commit to hanwen-pcluste/aws-parallelcluster-cookbook that referenced this pull request Feb 15, 2024
The bug was introduced in aws#2617

Signed-off-by: Hanwen <hanwenli@amazon.com>
hanwen-pcluste pushed a commit that referenced this pull request Feb 15, 2024
The bug was introduced in #2617

Signed-off-by: Hanwen <hanwenli@amazon.com>
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.

4 participants