Allow disabling of network activation (SC-307)#1048
Conversation
TheRealFalcon
force-pushed
the
unbringup-interfaces
branch
3 times, most recently
from
2e1d348 to
24023c3
Compare
|
This covers the functionality and an integration test for the failure case. Still missing unit tests and documentation for this. |
|
I added a unit test. @blackboxsw , curious if you have strong opinions about docs. This feels like something that should be documented, but we don't really have documentation for /etc/cloud/cloud.cfg. I could add a section for it, but this shouldn't be the only thing about it documented either. |
TheRealFalcon
changed the title
blackboxsw
left a comment
blackboxsw
left a comment
There was a problem hiding this comment.
Just a request on the integration test for a bit more verbose error reporting. I think maybe logs could be added to doc/rtd/topics/network-config.rst.
| self.assertIn(log, self.stderr.getvalue()) | ||
|
|
||
|
|
||
| class TestShouldBringUpInterfaces: |
There was a problem hiding this comment.
Thanks for the test coverage here. It's functional for the additional function created. Since main isn't covered well with unit tests, this makes things tough to instrument unit tests for this interaction so I think its fine we rely on the integration test here.
| network_activator = activators.select_activator() | ||
| network_activator.bring_up_all_interfaces(network_state) | ||
| else: | ||
| LOG.debug("Not bringing up newly configured network interfaces") |
There was a problem hiding this comment.
Good log tracers here for quick triage in failure cases if needed.
| """Test that the network is not activated during init mode.""" | ||
| _setup_custom_image(session_cloud) | ||
| with session_cloud.launch() as client: | ||
| assert client.execute('systemctl is-active google-guest-agent').ok |
There was a problem hiding this comment.
In the event of failure the assert isn't going to be super helpful in triage. Can we do something like this?
| assert client.execute('systemctl is-active google-guest-agent').ok | |
| result = client.execute('systemctl is-active google-guest-agent') | |
| if not result.ok: | |
| raise AssertionError("google-guest-agent is not active:\n%s" % result.stdout) |
There was a problem hiding this comment.
@TheRealFalcon your commit message too should reference LP: #1938299
| _setup_custom_image(session_cloud) | ||
| with session_cloud.launch() as client: | ||
| assert client.execute('systemctl is-active google-guest-agent').ok | ||
| result = client.execute('systemctl is-active google-guest-agent') |
There was a problem hiding this comment.
Can this be systemctl status google-guest-agent.service ? it retuns non-zero (3) in failed/dead state
There was a problem hiding this comment.
On a quick note: whilst status works, I wonder if leveraging is-active a better way to go about it?
There was a problem hiding this comment.
@utkarsh2102 , I think the idea was to get the additional information to stdout from the status call if it fails, vs just something like "inactive" when checking is-active. Why do you think is-active would be better?
In canonical#919 (81299de), we refactored some of the code used to bring up networks across distros. Previously, the call to bring up network interfaces during 'init' stage unintentionally resulted in a no-op such that network interfaces were NEVER brought up by cloud-init, even if new network interfaces were found after crawling the metadata. The code was altered to bring up these discovered network interfaces. On ubuntu, this results in a 'netplan apply' call during 'init' stage for any ubuntu-based distro on a datasource that has a NETWORK dependency. On GCE, this additional 'netplan apply' conflicts with the google-guest-agent service, resulting in an instance that can no be connected to. This commit adds a 'disable_network_activation' option that can be enabled in /etc/cloud.cfg to disable the activation of network interfaces in 'init' stage. LP: #1938299
TheRealFalcon
force-pushed
the
unbringup-interfaces
branch
from
70b8438 to
0e02eaf
Compare
blackboxsw
left a comment
blackboxsw
left a comment
There was a problem hiding this comment.
Excellent thank you for this @TheRealFalcon Validated integration test runtomorrow morn.
Integration test will cover our basic case for asserting google-guest-agent is running with the features and disable flag. Also confirmed failure path without. I think we might want a followup to ensure we setup default project-wide users for our project so we can confirm that they are also properly created. This is a bit broader in scope than we need to handle at the moment.
…age (#1796) This updates Ubuntu import to use the workaround from Canonical in canonical/cloud-init#1048.
3 participants
Proposed Commit Message
Additional Context
Test Steps
Checklist: