-
Notifications
You must be signed in to change notification settings - Fork 64
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
Don't mark machine ready=false when internalIP is known #265
Conversation
Signed-off-by: David Vossel <davidvossel@gmail.com>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: davidvossel The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/ok-to-test |
@@ -349,6 +359,15 @@ func (r *KubevirtMachineReconciler) reconcileNormal(ctx *context.MachineContext) | |||
return ctrl.Result{}, nil | |||
} | |||
|
|||
func machineHasKnownInternalIP(kubevirtMachine *infrav1.KubevirtMachine) bool { | |||
for _, addr := range kubevirtMachine.Status.Addresses { | |||
if addr.Type == clusterv1.MachineInternalIP && addr.Address != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should skip ipv6 link local
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That made sense to me at first, but after discussing this further I'm not 100% sure. It's theoretically possible someone might be using this in a network where link local addresses for VMs technically work. It's just that the use case we're most familiar with (vms on pod network) wouldn't use link local.
/lgtm |
We've seen instances where the vmi.Status.Interfaces list temporarily clears and stops reporting the default instance IP. This occurs when the qemu guest agent can not be contacted, such as during an internal soft reboot. It also occurs for reasons we do not 100% understand yet.
Since we know this vmi.Status.Interfaces field is dependent on the qemu guest agent, there will be times where the agent is unavailable. To smooth over those time periods, capk now only reports IP changes on the node when we're certain the IP has actually changed, and does not clear out the internal IP when no default ip is reported on the vmi status.