Ubuntu/jammy pre-upload review of 24.3.1 against release package version 24.2-0ubuntu1~22.04.1 #28
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The /etc/hostname.* files should have the mtu on a separate line otherwise it gives error: ifconfig: mtu: bad value The lines are executed in order by ifconfig and mtu should be on it's own line. Fixes: canonicalGH-5413
* Remove `unittest` constructs and remove base classes. * Replace tests that don't test things with tests that do * Add fstab and mounts combinations test
There are no call sites requesting not decoding the environment vars. This change decodes then always, simplifying typing and logic.
Instead of a broad try/except, do properly check for conditions that invalidate a mount location.
Support for jumbo frames requires that the underlying physical interfaces and the parent bond interface all have the larger MTU configured, not just the physical interfaces.
…canonical#5501) At least one of (or both) 'baseurl' or 'metalink' should be provided for yum repository specification. Add schema changes to enforce it. Without this, with just 'metalink' property set, one would get the schema validator error \--- Error: Cloud config schema errors: yum_repos.epel-release: 'baseurl' is a required property \--- Signed-off-by: Ani Sinha <[email protected]>
On systemd, services are started by PID 1. When this doesn't happen, cloud-init is in an unknown run state and should warn the user. Reorder pid log to be able to reuse Distro information. Add docstring deprecating util.is_Linux().
…5209) Implement verify_clean_boot() to ignore certain expected logs in a platform-specific way.
…anonical#5209) Ensure ignore_warnings=True or ignore_errors=True is honored and not overridden by supplemental warning texts appended.
…l#5209) Avoid using warning level messages as there may be some use-cases in the wild that need to invoke cloud-init boot stages after boot for some reason unknown to upstream. Provide a detailed warning message informing admins to file issues against cloud-init to better represent those feature needs before dropping this feature altogether.
…onical#5504) Commit 604d80e introduced assertions expecting exit 2 from the CLI when calling cloud-init init --local. Revert this test assertion as only cloud-init status command exits (2) on deprecations/warnings. Invoking cloud-init's boot stages on the commmand line will only exit 1 if critical errors are encountered to avoid degrading overall systemd health as seen from cloud-init systemd units. When cloud-init boot stages encounter recoverable_errors of any type, there is no need to exit non-zero as those deprecation logs are not-critical to the health of the system as a whole.
It is pretty consistently failing due to canonical#5373 with no fix in sight.
Ensure DNS server addresses are parsed from the proper location of network_data.json Fixes canonical#5386 Co-authored-by: Alberto Contreras <[email protected]>
If DNS information is added to a NetworkManager managed interface where the given protocol family is disabled, NetworkManager will be unable to activate the interface. canonical#5387
'mirrorlist' config can be specified instead or along with 'baseurl' in the yum repository config. Add support for specifying mirrorlist instead of 'baseurl'. Fixes canonicalGH-5520 Signed-off-by: Ani Sinha <[email protected]>
9929a00 added the ability to used a cached datasource when none is found. This was supposed to be per-datasource, but the lack of cache cleaning got applied universally. This commit makes it so cache will be cleaned as it was before if fallback isn't implemented in datasource. Fixes canonicalGH-5486
This is useful for logs we want hidden by default but can be turned on via configuration.
Switch to pathlib where appropriate and call consistently
…5515) With this change, the following config in cloud.cfg.d/ will select NoCloud in network stage. ``` datasource_list: [ GCE, NoCloud, None ] datasource: NoCloud: seedfrom: http://0.0.0.0:8000/ ``` Previously a two or less datasources in the datasource_list were required to get this behavior, which was undocumented and not intuitive. The ds-identify already allowed inline user-data and meta-data to trigger detection. Add ds-identify unittests for seedfrom and inline user-data. Add DataSourceNoCloud.ds_detect() unittests for seedfrom and inline user-data.
The nocloud datasource logs messages that are sometimes confused by users for errors. Clarify them. Also, remove redundant information from the logs: - simplify log wording - only include seed and dsmode information in nocloud string when non-default values are used
Align integration test with c28092f.
Adapt to new annotation formating from a2193da.
…onical#5636) Directly calling execute("cloud-init clean --logs --reboot") on an integration instances also involves awaiting a new boot id upon next interaction with with instance to ensure a reboot has actually taken place already on this target machine. Slow responding test instances/platforms may not completed the shutdown restart sequence yet when trying to iteract with an immediate blocking call to execut("cloud-init status --wait") which may exit early if accessing the prior instance boot before the reboot occurred. It is preferable to use inspect /proc/sys/kernel/random/boot_id before issuing a reboot request and block until a delta is seen in boot_id. This blocking wait on reboot and new boot_id is encapsulated inside pycloudlib.BaseInstance.restart which will inspect /proc/sys/kernel/random/boot_id before restart and block until a delta in boot_id across the requested restart. Fix test_status_block_through_all_boot_status to call instance.clean() and restart() to ensure we do not beat the instance reboot race with our post-boot assertions.
Add PPS support for azure-proxy agent and improve error logging.
…nical#5633) Do not treat the emptiness of .cloud-init/ as an error in the logs if agent.yaml is present. Fixes canonicalGH-5632
…ply (canonical#5622) Perform the same steps that cloud-init daily recipe builds performs to assert any packaging branch updates will not break daily builds due to quilt patch apply issues. Steps of daily build recipe reflected in this workflow: - checkout main - merge packaging branch topmost commit - quilt push -a - run unittests (via tox -e py3) - quilt pop -a
…cal#5641) Reintroduce strict assert that cloud-init's cert in userdata is the only root cert defined on the platform. Google guest agent was installed a secondary root cert in ca_certifications.crt for a period of time and this was determined to be less than ideal practice. Allow cloud-init's integration tests to remain strict validation of cert checksum to provide a signal if other platforms or agents attempt to extend or alter the system-wide CA.
…d field (canonical#5355) Currently cc_user_groups assumes that "useradd" never locks the password field of newly created users. This is an incorrect assumption. Change add_user (in both __init__.py and alpine.py) to explicitly call either lock_passwd or unlock_passwd at all times to achieve the desired final result. For existing users with empty or empty locked passwords, no password unlock will be performed and warnings will be issued. To support empty password validation, provide functionality to parse /etc/shadow and /var/lib/extrausers/shadow to assert existing users do not have empty passwords before unlocking. Additionally in this commit: - add NetworkBSD.ifs property to avoid subp side-effect in ___init__ which calls ifconfig -a at every instance initialization Useradd background: From the useradd manpage: '-p, --password PASSWORD The encrypted password, as returned by crypt(3). The default is to disable the password.' That is, if cloud-init runs 'useradd' but does not pass it the "-p" option (with an encrypted password) then the new user's password field will be locked by "useradd". cloud-init only passes the "-p" option when calling "useradd" when user-data specifies the "passwd" option for a new user. For user-data that specifies either the "hashed_passwd" or "plain_text_passwd" options instead then cloud-init calls "useradd" without the "-p" option and so the password field of such a user will be locked by "useradd". For user-data that specifies "hashed_passwd" for a new user then "useradd" is called with no "-p" option, so causing "useradd" to lock the password field, however then cloud-init calls "chpasswd -e" to set the encrypted password which also results in the password field being unlocked. For user-data that specifies either "plain_text_passwd" for a new user then "useradd" is called with no "-p" option, so causing "useradd" to lock the password. cloud-init then calls "chpasswd" to set the password which also results in the password field being unlocked. For user-data that specifies no password at all for a new user then "useradd" is called with no "-p" option, so causing "useradd" to lock the password. The password field is left locked. In all the above scenarios "passwd -l" may be called later by cloud-init to enforce "lock_passwd: true"). Conversely where "lock_passwd: false" applies the above "usermod" situation (for "hash_passwd", "plain_text_passwd" or no password) means that newly created users may have password fields locked when they should be unlocked. For Alpine, "adduser" does not support any form of password being passed and it always locks the password field (the same point applies about password field being unlocked when/if "chpasswd" is called). Therefore in some situations (i.e. no password specified in user-data) the password needs to be unlocked if "lock_passwd: false".
Bump the version in cloudinit/version.py to 24.3 and update ChangeLog.
Drop unnecessary environment variable. Fixes canonicalGH-5648
Bump the version in cloudinit/version.py to 24.3.1 and update ChangeLog.
patches: debian/patches/cli-retain-file-argument-as-main-cmd-arg.patch debian/patches/no-nocloud-network.patch debian/patches/revert-551f560d-cloud-config-after-snap-seeding.patch
panlinux
reviewed
Sep 6, 2024
|
I'm having a hard time establishing that these changes are the same-ish than the noble ones. git range-diff is failing me and not showing anything useful. Maybe because the commits were not in the same order. For the record, I'm doing: The output shows nonsense like A=A: Maybe because the commits are in a different order. If that's the case, it might be easier to review this in the unapproved queue, comparing it to noble, content-wise, and not commit-wise. I'll try on Monday again. |
|
Since the upstream version is the same as noble, I'll diff the debian/ changes only. |
panlinux
reviewed
Sep 9, 2024
| mydata["user-data"] = ud | ||
| mydata["vendor-data"] = vd | ||
| - mydata["network-config"] = network | ||
| + mydata["network-config"] = None |
There was a problem hiding this comment.
In noble, this hunk is different. There we just don't set mydata["network-config"]:
@@ -199,7 +199,6 @@
)
mydata["user-data"] = ud
mydata["vendor-data"] = vd
- mydata["network-config"] = network
found.append(seedfrom)
# Now that we have exhausted any other places merge in the defaultsThere was a problem hiding this comment.
And this patch is also missing this other hunk from noble:
--- a/cloudinit/sources/DataSourceNoCloud.py
+++ b/cloudinit/sources/DataSourceNoCloud.py
@@ -190,7 +190,7 @@
# This could throw errors, but the user told us to do it
# so if errors are raised, let them raise
- md_seed, ud, vd, network = util.read_seeded(seedfrom, timeout=None)
+ md_seed, ud, vd, _ = util.read_seeded(seedfrom, timeout=None)
LOG.debug("Using seeded cache data from %s", seedfrom)
# Values in the command line override those from the seed
This one isn't that important, but the previous one seems a bigger difference.
There was a problem hiding this comment.
Those hunks are roughly inert in either case because mydata is initialized as "network-config": None, but it's better not to have a quilt patch diff that we have to look at across series and I had refreshed one patch and not refreshed the other completely. Thanks for pointing it out, I'll bring jammy into the fold w/ the same quilt patch.
panlinux
reviewed
Sep 9, 2024
| @@ -67,7 +67,7 @@ Last-Update: 2024-02-14 | |||
|
|
|||
There was a problem hiding this comment.
Is this patch needed in noble? Because it's not there.
There was a problem hiding this comment.
Noble doesn't need that because 551f560 already released to noble before noble originally published in version 24.1_6. So no need for us to revert that after the fact.
panlinux
suggested changes
Sep 9, 2024
Revert remaning functional references to cloud-init-network service which will not exist on stable releases.
blackboxsw
force-pushed
the
ubuntu/jammy
branch
from
7219a3f to
d1889c3
Compare
|
Thank you @panlinux. I'll close this branch as approved and merge the upstream PR canonical#5678 once our upload gets accepted into -proposed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Review blocks upstream PR canonical#5678
cc: @panlinux