Skip to content

Conversation

@a-dubs
Copy link
Contributor

@a-dubs a-dubs commented Mar 13, 2025

Proposed Commit Message

N/A

Additional Context

Test Steps

Merge type

  • [] Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

@github-actions github-actions bot added the documentation This Pull Request changes documentation label Mar 13, 2025
Copy link
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.

+1 to the approach here!

It looks like the key being used by netplan is deprecated, and there's an old PR to update it in netplan.

I'm tempted to call this KeepConfiguration in v1.

@blackboxsw blackboxsw force-pushed the add-critical-networking-config-option branch from 02c8b8d to adc8b8b Compare March 19, 2025 19:51
@TheRealFalcon TheRealFalcon added the CLA not signed The submitter of the PR has not (yet) signed the CLA label Mar 19, 2025
@a-dubs a-dubs marked this pull request as ready for review March 24, 2025 18:25
@a-dubs a-dubs force-pushed the add-critical-networking-config-option branch 2 times, most recently from ae03d70 to 94d007a Compare March 24, 2025 19:01
@a-dubs a-dubs mentioned this pull request Mar 25, 2025
1 task
@a-dubs a-dubs force-pushed the add-critical-networking-config-option branch from e243d81 to f7aac83 Compare March 25, 2025 17:12
@a-dubs a-dubs changed the title Add 'critical' networking config option to internal v1 and v2 networking config Add 'keep_configuration' networking config option to internal v1 networking config Mar 25, 2025
@a-dubs
Copy link
Contributor Author

a-dubs commented Mar 25, 2025

@TheRealFalcon this is ready for re-review. git history is a mess but will cleanup once reviewing is done.

@a-dubs a-dubs requested a review from TheRealFalcon March 25, 2025 18:48
@blackboxsw blackboxsw removed the CLA not signed The submitter of the PR has not (yet) signed the CLA label Mar 25, 2025
Copy link
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.

I left a few minor inline comments (and one somewhat major question), but it generally looks good to me!

Copy link
Contributor Author

@a-dubs a-dubs left a comment

Choose a reason for hiding this comment

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

@TheRealFalcon made ur changes!

@a-dubs a-dubs force-pushed the add-critical-networking-config-option branch 2 times, most recently from 6b3953d to 857d727 Compare March 26, 2025 19:54
Copy link
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.

LGTM! You mentioned possibly restructuring the commits, so let me know when and how you wanna merge.

@a-dubs a-dubs marked this pull request as draft March 27, 2025 14:29
@a-dubs
Copy link
Contributor Author

a-dubs commented Mar 31, 2025

aaaaaand this change is no longer necessary (for the time being) due to similar functionality existing in initramfs-tools on noble+ (ubuntu 24.04 and forward).

Closing! :P

@TheRealFalcon
Copy link
Contributor

Continued in #6307

@a-dubs
Copy link
Contributor Author

a-dubs commented Jul 16, 2025

WE BACK
tenor
Issue #6306 for context

@a-dubs a-dubs reopened this Jul 16, 2025
@a-dubs a-dubs marked this pull request as ready for review July 16, 2025 15:16
@a-dubs
Copy link
Contributor Author

a-dubs commented Jul 16, 2025

Clearly the commits need squashed. @TheRealFalcon do you reckon it would be best to squash into one commit via the PR desc? or is it worth doing a couple commits (like one for the schema change and then one for the oracle impl)?

@TheRealFalcon
Copy link
Contributor

@a-dubs , maybe one for the change, one for Oracle. But I have no problem squashing into 1 either.

@a-dubs
Copy link
Contributor Author

a-dubs commented Jul 16, 2025

@a-dubs , maybe one for the change, one for Oracle. But I have no problem squashing into 1 either.

Sweet! Will do then.

Also, @TheRealFalcon should I be adding/updating some docs somewhere for this change?

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.

I was wondering whether we need to document iSCSI keep_configuration behavior in doc/rtd/refefence/datasources/oracle.rst. But I think that is an implementation detail that's not worth publicizing.

@a-dubs a-dubs force-pushed the add-critical-networking-config-option branch 3 times, most recently from 8a65570 to f837fcf Compare July 17, 2025 14:09
a-dubs added a commit to a-dubs/cloud-init that referenced this pull request Jul 18, 2025
This is necessary for Oracle Baremetal instances on IPv6-only networks
which rely on the iscsi connection. Without this, after shutting down
the instance, the instance is not recoverable.

This setting will be automatically set for all iscsi instances via the
Oracle Datasource based on metadata/config files left behind by
initramfs during boot on ISCSI instances.

PR: canonical#6097
@a-dubs a-dubs force-pushed the add-critical-networking-config-option branch from f837fcf to d253a84 Compare July 18, 2025 11:38
@a-dubs
Copy link
Contributor Author

a-dubs commented Jul 18, 2025

@TheRealFalcon okay I have split the changes into to two commits and added the PR # reference to the footers of the commit messages!

@blackboxsw I made the easy change to the rtd you requested. So this should all be good to go hopefully.

@TheRealFalcon
Copy link
Contributor

@uhryniuk , were you interested in testing this behavior on Oracle?

@uhryniuk
Copy link

@TheRealFalcon Yeah, if you don't mind were going to fire off a bit more testing but by EOW or hopefully earlier we should be ready to roll. I'll let you know when it's completed!

a-dubs added a commit to a-dubs/cloud-init that referenced this pull request Jul 25, 2025
This is necessary for Oracle Baremetal instances on IPv6-only networks
which rely on the iscsi connection. Without this, after shutting down
the instance, the instance is not recoverable.

This setting will be automatically set for all iscsi instances via the
Oracle Datasource based on metadata/config files left behind by
initramfs during boot on ISCSI instances.

PR: canonical#6097
@a-dubs a-dubs force-pushed the add-critical-networking-config-option branch from d253a84 to 8057b7a Compare July 25, 2025 21:49
@uhryniuk
Copy link

@TheRealFalcon We finished up our testing, we're good to go. @a-dubs just pushed some more changes to fix the failing keep_configuration OCI integration test. The PR CI seems to have failed after the new commit though, but it appears unrelated.

@TheRealFalcon
Copy link
Contributor

@a-dubs , can you push a rebase? It should fix the test error.

a-dubs added 2 commits July 29, 2025 10:35
This config option is only implemented for the Netplan renderer.

It is used to designate the connection as 'critical to the system', meaning that
special care will be taken not to release the assigned IP when the daemon is
restarted.

PR: 6097
This is necessary for Oracle Baremetal instances on IPv6-only networks
which rely on the iscsi connection. Without this, after shutting down
the instance, the instance is not recoverable.

This setting will be automatically set for all iscsi instances via the
Oracle Datasource based on metadata/config files left behind by
initramfs during boot on ISCSI instances.

PR: canonical#6097
@a-dubs a-dubs force-pushed the add-critical-networking-config-option branch from 8057b7a to 757dcfe Compare July 29, 2025 14:35
@a-dubs
Copy link
Contributor Author

a-dubs commented Jul 29, 2025

@TheRealFalcon done!

Copy link
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.

LGTM!

@TheRealFalcon TheRealFalcon merged commit 0fec2e2 into canonical:main Jul 29, 2025
22 checks passed
DarkPhily pushed a commit to hetznercloud/cloud-init that referenced this pull request Sep 2, 2025
This is necessary for Oracle Baremetal instances on IPv6-only networks
which rely on the iscsi connection. Without this, after shutting down
the instance, the instance is not recoverable.

This setting will be automatically set for all iscsi instances via the
Oracle Datasource based on metadata/config files left behind by
initramfs during boot on ISCSI instances.

PR: canonical#6097
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation This Pull Request changes documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants