Skip to content
This repository has been archived by the owner on Mar 27, 2023. It is now read-only.

Add a DeployedCondition for the VirtualMachine object #82

Closed
wants to merge 1 commit into from

Conversation

sreyasn
Copy link
Contributor

@sreyasn sreyasn commented Aug 11, 2021

This change adds a conditionType for the deployment status of a virtualmachine on
the infrastructure provider.

@sreyasn sreyasn requested review from bryanv and janitha August 12, 2021 00:08
api/v1alpha1/condition_consts.go Outdated Show resolved Hide resolved
This change adds a conditionType for the deployment status of a virtualmachine on
the infrastructure provider.
@@ -37,6 +37,14 @@ const (
VirtualMachineImageNotFoundReason = "VirtualMachineImageNotFound"
)

const (
// DeployedCondition exposes the deployment status of a VirtualMachine on the infrastructure provider.
DeployedCondition ConditionType = "Deployed"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I understand what this means. The VM hardware deployed successfully? The virtual hardware failed? The guest customization succeeded / failed? Both? I am not sure this makes sense as-is. Let's revisit this a bit more. I don't know that aggregate conditions make sense, and we already have a guest customization condition, so is deploy separate or a superset?

Copy link
Contributor Author

@sreyasn sreyasn Aug 18, 2021

Choose a reason for hiding this comment

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

Deploy is separate, It's synonymous to Created/Create in VM operator.

guest customization is triggered in our Update path after Create/Deploy passes.

Copy link
Contributor

@akutz akutz Aug 18, 2021

Choose a reason for hiding this comment

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

Deploy is separate, It's synonymous to Created/Create in VM operator.

The issue is that "deploy" is vague enough to be taken multiple ways. Plus, conditions exist as a way to be more distinct than allowed by a top-level status field such as phase. So let's come up with a better word than deployed as it seems to generic and could be mistaken to mean guest customization as well.

FWIW, here is the latest community discussion around the purpose of conditions.

DeployedCondition ConditionType = "Deployed"

// DeployFailedReason (Severity=Error) documents that the deployment of the VirtualMachine was not successful.
DeployFailedReason = "DeployFailed"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

@sreyasn sreyasn closed this Sep 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants