add mariner support #1780
add mariner support #1780
Conversation
| } | ||
|
|
||
|
|
||
| class TestMariner(CiTestCase): |
There was a problem hiding this comment.
For future reference, I'd recommend new tests use pytest rather than unittest where practical.
cloudinit/distros/mariner.py
Outdated
| """ | ||
|
|
||
|
|
||
| class Distro(distros.Distro): |
There was a problem hiding this comment.
Given that this class is almost entirely copied from the Photon distro class, why not just have the Distro class here inherit from photon.Distro and override the small pieces necessary?
There was a problem hiding this comment.
Totally make senses. I am working with my team to dig into this and find out what exactly are required for mariner.
There was a problem hiding this comment.
change as suggested to inherit from photon and override few methods
config/cloud.cfg.tmpl
Outdated
| @@ -99,11 +99,11 @@ cloud_init_modules: | |||
| - set_hostname | |||
| - update_hostname | |||
| - update_etc_hosts | |||
| {% if variant in ["alpine", "photon"] %} | |||
| {% if variant in ["alpine", "mariner", "photon"] %} | |||
There was a problem hiding this comment.
I see that you're basically copying whatever Photon does in this file, but does Mariner work exactly the same way? Is the resolv_conf module needed? Is there a reason to disable ca-certs and ssh-import-id?
There was a problem hiding this comment.
Working with my team to confirm these. will push a new commit.
There was a problem hiding this comment.
After discussion, we decided to leave those modules enabled and give the abilities to run those modules to users as long as they don't bring any side effects when no detailed configurations are specified under the module.
Proposed Commit Message
Additional Context
CBL-Mariner is an internal Linux distribution for Microsoft’s cloud infrastructure and edge products and services.
Details can you found here: CBL-Mariner
Test Steps
Running tox -e do_format and tox command to test
Checklist: