Skip to content

Add Rocky Linux support to cloud-init#906

Merged
TheRealFalcon merged 5 commits into
canonical:masterfrom
nazunalika:master
May 25, 2021
Merged

Add Rocky Linux support to cloud-init#906
TheRealFalcon merged 5 commits into
canonical:masterfrom
nazunalika:master

Conversation

@nazunalika
Copy link
Copy Markdown
Contributor

Proposed Commit Message

summary: Add support for Rocky Linux

Rocky Linux is a RHEL-compatible distribution so all changes that have
been made should be trivial.

Additional Context

https://rockylinux.org

We currently have RC1 out (for version 8.3) and currently working on 8.4. We
had to redo our patches for 8.4 as it was rebased. We realized an upstream PR
was not made for our distribution, hence this submission.

Please let me know if I made a mistake anywhere throughout this PR and I
will try to correct it.

Thank you!

Test Steps

Checklist:

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly
  • I have updated or added any documentation accordingly

@blackboxsw blackboxsw self-assigned this May 19, 2021
Copy link
Copy Markdown
Contributor

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Thanks for this submission. Overall this looks good. A few notes...

Most of the lists you added rocky to were in alphabetical order. Instead of adding rocky to the end of the list, can you add it to keep the ordering alphabetical?

One of the unit tests you wrote is failing on CI. It's because of the parsing of /etc/redhat-release is expecting a codename after the version number. Would it be possible to update the implementation to make the codename optional?

I suggested a change inline for the other failing unit test.

nazunalika and others added 2 commits May 24, 2021 12:34
* Change alphabetical order on some lines
* Version code name decided from releng, add it in
@nazunalika
Copy link
Copy Markdown
Contributor Author

I believe I addressed the alphabetical order lists and also the code name. We added a code name recently but I didn't provide an updated set of info. Hopefully that addressed that issue there. I couldn't find your suggestion in the modified file list. Or perhaps I've looked in the wrong place.

Copy link
Copy Markdown
Contributor

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Hmmm, weird...must have put that suggestion in a different tab then I closed or something. Should be here now.

Comment thread tests/unittests/test_cli.py Outdated
Comment on lines +227 to +228
('**Supported distros:** almalinux, alpine, centos, debian, '
'fedora, rocky'),
Copy link
Copy Markdown
Contributor

@TheRealFalcon TheRealFalcon May 25, 2021

Choose a reason for hiding this comment

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

This is checking that the list in cc_ntp.py matches some documentation output. Unfortunately, it hasn't been updated in a while, but this should fix it.

Suggested change
('**Supported distros:** almalinux, alpine, centos, debian, '
'fedora, rocky'),
('**Supported distros:** almalinux, alpine, centos, debian, '
'fedora, opensuse, rhel, rocky, sles, ubuntu'),

Copy link
Copy Markdown
Contributor

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Looks good now, thanks!

@TheRealFalcon TheRealFalcon merged commit 7c1d27b into canonical:master May 25, 2021
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.

3 participants