Support for AIX #4713

AkashKTripathi · 2023-12-21T13:44:29Z

We are using this Issue to push the Code changes that are needed to provide IBM AIX OS support for the cloud-init.

In multiple location, where the command are specific to IBM AIX, We have inserted the if condition to check the underlying OS.
We are running AIX specific command only if the AIX is running on that system.

igalic

this is a first sweep across the changes. some general comments:

I think this is a case where more than one commit would be very appreciated

subp shouldn't be used with qualified paths, unless there's good reason to do so

igalic · 2023-12-21T14:38:12Z

cloudinit/config/cc_growpart.py

@@ -15,6 +15,7 @@
 import os.path
 import re
 import stat
+import platform


rather than modify every single module to reject AIX, please extend util.py to extend system_info() and add an is_AIX() function

igalic · 2023-12-21T14:47:42Z

cloudinit/config/cc_keys_to_console.py

@@ -19,6 +20,7 @@
 from cloudinit.settings import PER_INSTANCE

 # This is a tool that cloud init provides
+HELPER_TOOL_TPL_AIX = "/opt/freeware/lib/cloud-init/write-ssh-key-fingerprints"


there's no need to modify this, much less hard code it.
all you're doing is calling ./setup.py with different parameters

igalic · 2023-12-21T14:50:46Z

cloudinit/config/cc_power_state_change.py

@@ -24,6 +25,7 @@
 frequency = PER_INSTANCE

 EXIT_FAIL = 254
+AIX = 0


please don't use modifiable globals
once an @lru_cached util.is_AIX() this won't especially be needed

igalic · 2023-12-21T14:52:19Z

cloudinit/config/cc_power_state_change.py

@@ -94,6 +96,9 @@ def givecmdline(pid):
            line = output.splitlines()[1]
            m = re.search(r"\d+ (\w|\.|-)+\s+(/\w.+)", line)
            return m.group(2)
+        elif AIX:
+            (ps_out, _err) = subp.subp(["/usr/bin/ps", "-p", str(pid), "-oargs="], rcs=[0, 1])


unless there is a ps in a standard path earlier than this, it shouldn't be necessary to give it the full path

igalic · 2023-12-21T14:57:14Z

cloudinit/config/cc_set_passwords.py

@@ -154,18 +155,26 @@ def handle_ssh_pwauth(pw_auth, distro: Distro):
        return

    if distro.uses_systemd():


does AIX use systemd?

igalic · 2023-12-21T16:13:52Z

cloudinit/helpers.py

@@ -306,17 +307,23 @@ class Paths(persistence.CloudInitPickleMixin):
    def __init__(self, path_cfgs: dict, ds=None):
        self.cfgs = path_cfgs
        # Populate all the initial paths
-        self.cloud_dir: str = path_cfgs.get("cloud_dir", "/var/lib/cloud")
+        if platform.system().lower() == "aix":
+            self.cloud_dir: str = path_cfgs.get("cloud_dir", "/opt/freeware/var/lib/cloud")


this shouldn't be done here, see #4677

igalic · 2023-12-21T16:16:43Z

cloudinit/settings.py

+if platform.system().lower() == "aix":
+    CLOUD_CONFIG = "/opt/freeware/etc/cloud/cloud.cfg"
+else:    
+    CLOUD_CONFIG = "/etc/cloud/cloud.cfg"


please see how we do this for FreeBSD

igalic · 2023-12-21T16:19:37Z

cloudinit/sources/DataSourceNoCloud.py

-        devlist = list(set(fslist) & set(label_list))
-        devlist.sort(reverse=True)
+        if platform.system().lower() == "aix":
+            devlist = aix_util.find_devs_with("cd0")


i know the structure in util.py isn't great, but, why aren't you extending that, rather tumač than touching every DataSource separately?

igalic · 2023-12-21T16:22:16Z

config/cloud.cfg

@@ -0,0 +1,102 @@
+# The top level settings are used as module


this file needs to go

igalic · 2023-12-21T16:25:04Z

sysvinit/aix/cloud-config

+
+# If there exist sysconfig/default variable override files use it...
+[ -f /etc/sysconfig/cloud-init ] && . /etc/sysconfig/cloud-init
+[ -f /etc/default/cloud-init ] && . /etc/default/cloud-init


other init systems also allow disabling with the existence of a file

for context

holmanb

Hey @AkashKTripathi,

Thanks for this contribution to cloud-init. As described in the contribution guide, you will need to sign the CLA before this contribution can be accepted.

A few initial comments on the code:

This is a large contribution; props for putting this together! There is a fair amount of work still before this can be merged into upstream cloud-init. I'll outline below what needs to happen next (assuming CLA gets signed). If you have any questions, please don't hesitate to ask.

Validating this contribution

If you are able to provide instructions for running AIX locally in a VM or in a cloud, that would greatly improve the upstream developers' abilities to verify and review this and future contributions to the AIX portions of cloud-init code. Our support matrix of clouds and virtual machine platforms is pretty long. A pointer to some working instructions on how to run AIX on Qemu would be very beneficial. I haven't found anything authoritative or that didn't require building Qemu from source, but then again I didn't look very hard.

There are syntactical errors in this code. That makes me wonder if this version has ran on AIX. Please make sure that this code can at least boot and run on an instance of AIX. Once it can, please attach the logs (/var/log/cloud-init.log and /var/log/cloud-init-output.log). This will help us to review and verify the functionality of this code. We typically look for tracebacks and errors in the logs for indicators of issues.

Tests

Numerous existing tests are failing. I just ran the CI job so that you can look at what failed. These can run from the source tree using tox, which automatically installs test dependencies via pip. For the formatting issues, tox -e do_format will fix most of them (we use black).

Linters and static analyzers

These test simple things like syntax and type checking.

tox -e ruff
tox -e mypy
tox -e pylint
tox -e check_format

Unit Tests

tox -e py3

Integration Tests

These are very powerful and there are many ways to use them. The easiest way is probably to install lxd and then run tests locally using the following command. I typically use Ubuntu but I have tested this method on a few other distributions as well.

CLOUD_INIT_SOURCE="IN_PLACE" tox -e integration-tests-ci

Once these integration tests all pass, your code will be ready for a more in depth review.

New Unit Tests

Kudos for adding your own unit tests. I haven't taken a close look yet, but that's a great sign for code quality.

A quick skim through the tests does reveal a few things to consider. I honestly don't know how they are supposed to work or how to run them. We use pytest for our tests, and invoke pytest using tox. These come with a bit of complexity, but they allow us to do lots of things powerfully and expressively: manage test dependencies automatically with pip, disable tests when they aren't supported by the platform that they run on, provide simple one line invocations to run various types of tests, etc, etc. I think we'll want at the very least to be using pytest for new tests so that they can be integrated into the rest of the test suite. This will allow us to disable these tests if, for example, they only work on AIX, but allow you to still run the distro-agnostic unittests when you run tox -e py3 on AIX. See this page in the docs for some pointers on pytest.

holmanb · 2024-01-04T02:53:16Z

cloudinit/cmd/main.py

+        # instead of optical disk to fetch network config. This check make sure 
+        # that customer always uses latest config data even if the instance-id 
+        # is matching
+        init.purge_cache(True)


One more comment:

This is a pretty big change to cloud-init. What justification do you have for it? Also, which datasources have you tested this on? Wouldn't this break any user using nocloud with only a CIDATA drive?

github-actions · 2024-01-19T00:21:33Z

Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close.

If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging TheRealFalcon, and he will ensure that someone takes a look soon.

(If the pull request is closed and you would like to continue working on it, please do tag TheRealFalcon to reopen it.)

holmanb · 2024-01-22T20:11:18Z

@AkashKTripathi Any update on this?

github-actions · 2024-02-06T00:19:54Z

Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close.

If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging TheRealFalcon, and he will ensure that someone takes a look soon.

(If the pull request is closed and you would like to continue working on it, please do tag TheRealFalcon to reopen it.)

holmanb · 2024-02-13T20:58:52Z

@AkashKTripathi Is there any interest in making this happen still?

AkashKTripathi · 2024-02-14T07:21:42Z

@AkashKTripathi Is there any interest in making this happen still?

Hi @holmanb
we are working on it. we will update you

github-actions · 2024-03-08T00:20:14Z

Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close.

If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging TheRealFalcon, and he will ensure that someone takes a look soon.

(If the pull request is closed and you would like to continue working on it, please do tag TheRealFalcon to reopen it.)

Support for AIX

eb04f0d

igalic suggested changes Dec 21, 2023

View reviewed changes

holmanb assigned holmanb and igalic Jan 2, 2024

holmanb requested changes Jan 4, 2024

View reviewed changes

holmanb reviewed Jan 4, 2024

View reviewed changes

github-actions bot added the stale-pr Pull request is stale; will be auto-closed soon label Jan 19, 2024

holmanb removed the stale-pr Pull request is stale; will be auto-closed soon label Jan 22, 2024

github-actions bot added the stale-pr Pull request is stale; will be auto-closed soon label Feb 6, 2024

github-actions bot closed this Feb 13, 2024

holmanb reopened this Feb 13, 2024

github-actions bot closed this Feb 22, 2024

holmanb removed the stale-pr Pull request is stale; will be auto-closed soon label Feb 23, 2024

holmanb reopened this Feb 23, 2024

github-actions bot added the stale-pr Pull request is stale; will be auto-closed soon label Mar 8, 2024

github-actions bot closed this Mar 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for AIX #4713

Support for AIX #4713

AkashKTripathi commented Dec 21, 2023

igalic left a comment

igalic Dec 21, 2023

igalic Dec 21, 2023

igalic Dec 21, 2023

igalic Dec 21, 2023

igalic Dec 21, 2023

igalic Dec 21, 2023

igalic Dec 21, 2023

igalic Dec 21, 2023 •

edited

Loading

igalic Dec 21, 2023

igalic Dec 21, 2023

holmanb Jan 2, 2024

holmanb left a comment •

edited

Loading

holmanb Jan 4, 2024

github-actions bot commented Jan 19, 2024

holmanb commented Jan 22, 2024

github-actions bot commented Feb 6, 2024

holmanb commented Feb 13, 2024

AkashKTripathi commented Feb 14, 2024

github-actions bot commented Mar 8, 2024

		@@ -154,18 +155,26 @@ def handle_ssh_pwauth(pw_auth, distro: Distro):
		return

		if distro.uses_systemd():

		@@ -0,0 +1,102 @@
		# The top level settings are used as module

Support for AIX #4713

Support for AIX #4713

Conversation

AkashKTripathi commented Dec 21, 2023

igalic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igalic Dec 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holmanb left a comment • edited Loading

Choose a reason for hiding this comment

A few initial comments on the code:

Validating this contribution

Tests

Linters and static analyzers

Unit Tests

Integration Tests

New Unit Tests

Choose a reason for hiding this comment

github-actions bot commented Jan 19, 2024

holmanb commented Jan 22, 2024

github-actions bot commented Feb 6, 2024

holmanb commented Feb 13, 2024

AkashKTripathi commented Feb 14, 2024

github-actions bot commented Mar 8, 2024

igalic Dec 21, 2023 •

edited

Loading

holmanb left a comment •

edited

Loading