sources/azure: report ready in local phase#1265
Merged
TheRealFalcon merged 4 commits intoFeb 15, 2022
Merged
Conversation
cjp256
force-pushed
the
azure-report-ready-local-phase
branch
from
92bd8dd to
d60fee1
Compare
Pre-provisioned instances report ready early in the local phase and again in the non-local phase, during setup(). Non-PPS only reports ready during non-local phase. Update the process to report ready during the local phase for all cases. Only attempt to do so if networking is up to prevent stalling boot. We've already waited at least 20 minutes for DHCP if we're provisioning, or 5 minutes for DHCP on normal boot requesting updated network configuration. - Extend _report_ready() with pubkey_info and raise exception on error to consolidate reporting done in _negotiate() and _report_ready(). - Remove setup(), moving relevant logic into crawl_metadata(). - Move remaining _negotiate() logic into _cleanup_markers() and _determine_wireserver_pubkey_info(). These changes effectively fix two issues that were present: (1) _negotiated is incorrectly set to True When failing to report ready. _negotiate() squashed the exception and the return value was not checked. This was probably masked due to the forced removal of obj.pkl on Ubuntu instances, but would be preferable once we start persisting it to prevent unnecessary re-negotiation. (2) provisioning media is not ejected for non-PPS _negotiate() did not pass iso_dev parameter when reporting ready. The host will ensure this operation takes place, but it is preferable to eject /dev/sr0 from within the guest when we're done with it. Lastly, this removes any need for lease file parsing as the wireserver addressed is tracked for ephemeral DHCP. A follow-up PR will remove this now-unused logic. Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
cjp256
force-pushed
the
azure-report-ready-local-phase
branch
2 times, most recently
from
31a8ea7 to
94bc44c
Compare
cjp256
force-pushed
the
azure-report-ready-local-phase
branch
2 times, most recently
from
76bcad5 to
ad709f6
Compare
Contributor
Author
|
@TheRealFalcon any chance can we get this one in prior to release? |
Contributor
|
I'll start reviewing it. |
Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
cjp256
force-pushed
the
azure-report-ready-local-phase
branch
from
ad709f6 to
37d3a35
Compare
TheRealFalcon
approved these changes
Feb 15, 2022
blackboxsw
reviewed
Feb 15, 2022
| self._ephemeral_dhcp_ctx = None | ||
| if not hasattr(self, "iso_dev"): | ||
| self.iso_dev = None | ||
| self._iso_dev = None |
Collaborator
There was a problem hiding this comment.
@TheRealFalcon @cjp256 Hrm, if my cached obj.pkl pickle already has self._iso_dev set to something other than none, this unpickle will clear it's value. Does this break us?
Contributor
Author
There was a problem hiding this comment.
It would not break anything. We don't want to persist it anymore :)
Closed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Pre-provisioned instances report ready early in the local phase and
again in the non-local phase, during setup(). Non-PPS only reports
ready during non-local phase.
Update the process to report ready during the local phase for all
cases. Only attempt to do so if networking is up to prevent stalling
boot. We've already waited at least 20 minutes for DHCP if we're
provisioning, or 5 minutes for DHCP on normal boot requesting updated
network configuration.
Extend _report_ready() with pubkey_info and raise exception
on error to consolidate reporting done in _negotiate() and
_report_ready().
Remove setup(), moving relevant logic into crawl_metadata().
Move remaining _negotiate() logic into _cleanup_markers() and
_determine_wireserver_pubkey_info().
These changes effectively fix two issues that were present:
(1) _negotiated is incorrectly set to True
When failing to report ready. _negotiate() squashed the exception and
the return value was not checked. This was probably masked due to the
forced removal of obj.pkl on Ubuntu instances, but would be preferable
once we start persisting it to prevent unnecessary re-negotiation.
(2) provisioning media is not ejected for non-PPS
_negotiate() did not pass iso_dev parameter when reporting ready. The
host will ensure this operation takes place, but it is preferable to
eject /dev/sr0 from within the guest when we're done with it.
Lastly, this removes any need for lease file parsing as the wireserver
addressed is tracked for ephemeral DHCP. A follow-up PR will remove
this now-unused logic.
Signed-off-by: Chris Patterson cpatterson@microsoft.com