Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

d/cloud-init.preinst: drop 99-disable-network-config.cfg in Oracle instances (SC-1414) #2014

Closed
wants to merge 0 commits into from

Conversation

aciba90
Copy link
Contributor

@aciba90 aciba90 commented Feb 10, 2023

DO NOT SQUASH

Proposed Commit Message

d/cloud-init.preinst: drop 99-disable-network-config.cfg in Oracle instances

Additional Context

https://warthogs.atlassian.net/browse/SC-1414
https://bugs.launchpad.net/cloud-init/+bug/1956788

@aciba90
Copy link
Contributor Author

aciba90 commented Feb 10, 2023

After approved, I will replicate this change in b, f, j series.

Per #2009 (comment), I am assuming we do not need to update debian/changelog.

@TheRealFalcon
Copy link
Member

I could be wrong, but I think the meaning behind that comment was that because Chad was changing something that only ever appeared in devel and never got released, there was no need to update the changelog because there was no meaningful difference to what was already mentioned in the changelog. I think this would need a changelog entry.

@blackboxsw blackboxsw self-assigned this Feb 14, 2023
Copy link
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

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

Yes @aciba90, @TheRealFalcon is correct here, This is a debian packaging preinst behavior change that represents a new change in behavior which we will release (and need to account for in d/changelog). The prior PR you referenced was one which already had a line in d/changelog that told of updates to d/patches/retain-netplan-world-readable.patch which accounted for the minor quilt patch fixes we needed to actually properly build. Since nothing was built or published to the daily PPA, there was no 'public' d/changelog entry needed to account for a public change in behavior.

In this PR, I think we need something like the following:

diff --git a/debian/changelog b/debian/changelog
index 535749cbd..0a93c1b36 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+cloud-init (22.4.2-0ubuntu4) UNRELEASED; urgency=medium
+
+  * d/cloud-init.preinst: Oracle to remove vestigial /etc/cloud.cloud.cfg.d/
+    99-disable-network-config.cfg because system config is now honored before
+    datasource config: (LP: #1956788)
+
+ -- Chad Smith <chad.smith@canonical.com>  Mon, 13 Feb 2023 17:27:04 -0700
+
 cloud-init (22.4.2-0ubuntu3) lunar; urgency=medium
 
   * Upstream snapshot based on upstream/main at bdc7b040.
diff --git a/debian/cloud-init.preinst b/debian/cloud-init.preinst
index cb2ffd97c..0dcef7c4f 100644
--- a/debian/cloud-init.preinst
+++ b/debian/cloud-init.preinst
@@ -182,8 +182,8 @@ cleanup_lp1552999() {
 }
 
 cleanup_oci_network_lp1956788() {
-   # Remove a previously ignored and not needed any more file to disable network config
-   # in Oracle instances.
+   # Remove vestigial /etc/cloud/cloud.cfg.d/99-disable-network-config.cfg
+   # from Oracle now that datasource honors system cfg above datasource cfg.
    grep DataSourceOracle /var/lib/cloud/instance/datasource > /dev/null 2>&1 || return 0
    rm -f /etc/cloud/cloud.cfg.d/99-disable-network-config.cfg
 }

I think we want to wait to land this officially until we can validate the related PR #1998.
Let's gate landing this until 1998 is approved/merged.

request changes for d/changelog

@TheRealFalcon
Copy link
Member

@blackboxsw, actually I think this one needs to land first. If #1998 lands first, then the file being removed in this PR to disable networking config will be honored over the DS config, so networking will be disabled on every launch. As is, this file is basically ignored because of the datasource ordering. Removing this file should have no effect because the datasource config (currently) comes first.

@TheRealFalcon
Copy link
Member

We should wait until after the 23.1 release to merge this.

@blackboxsw
Copy link
Collaborator

We should wait until after the 23.1 release to merge this.
Agreed, I'll finish review on #1998 and we'll just keep the PR unmerged until we clear 23.1 SRU

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.

None yet

3 participants