Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/v1alpha1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions api/v1alpha2/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions api/v1alpha3/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions api/v1alpha4/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions api/v1alpha5/virtualmachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1220,6 +1220,20 @@ type VirtualMachineGuestStatus struct {
GuestFullName string `json:"guestFullName,omitempty"`
}

// VirtualMachineProviderStatus describes the observed state of the
// VirtualMachine from the underlying infrastructure provider (vSphere/vCenter).
type VirtualMachineProviderStatus struct {
// +optional

// CreationTimestamp describes the timestamp when the VirtualMachine was
// created in the infrastructure provider. This value is retrieved
// from the VM's config.createDate field and is immutable once set.
//
// This field is only populated after the VM has been successfully created
// in vCenter and the creation timestamp is available.
CreationTimestamp *metav1.Time `json:"creationTimestamp,omitempty"`
}

// VirtualMachineStatus defines the observed state of a VirtualMachine instance.
type VirtualMachineStatus struct {
// +optional
Expand Down Expand Up @@ -1320,6 +1334,12 @@ type VirtualMachineStatus struct {

// +optional

// Provider describes the observed state of the VirtualMachine from
// the underlying infrastructure provider (vSphere/vCenter).
Provider *VirtualMachineProviderStatus `json:"provider,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I initially loved this approach, but the more I think about it, the more it is confusing. Nearly everything in status comes from the provider, so I'm not sure we need to nest this. I'm also not convinced this should not be a condition if the goal is to understand the lifecycle of the VM since all other timestamps are part of conditions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my thought process for why this wouldn't fit in the conditions:

  • the timestamp in the conditions represents when did the transition occur which I think will be confusing if for the case of an imported VM a condition timestamp is going to be older than the CR's creation timestamp itself.
  • the condition will always transition either none or exactly once.
  • I thought it was already a pattern to store such info in the status like status.LastRestartTime

I agree that most of the CR is an abstraction of the underlying provider so I am open to other names or not having it nested.


// +optional

// CurrentSnapshot describes the observed working snapshot of the VirtualMachine.
// This field contains the name of the current snapshot.
CurrentSnapshot *VirtualMachineSnapshotReference `json:"currentSnapshot,omitempty"`
Expand Down
24 changes: 24 additions & 0 deletions api/v1alpha5/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13215,6 +13215,22 @@ spec:
- PoweredOn
- Suspended
type: string
provider:
description: |-
Provider describes the observed state of the VirtualMachine from
the underlying infrastructure provider (vSphere/vCenter).
properties:
creationTimestamp:
description: |-
CreationTimestamp describes the timestamp when the VirtualMachine was
created in the infrastructure provider. This value is retrieved
from the VM's config.createDate field and is immutable once set.

This field is only populated after the VM has been successfully created
in vCenter and the creation timestamp is available.
format: date-time
type: string
type: object
rootSnapshots:
description: |-
RootSnapshots represents the observed list of root snapshots of
Expand Down
18 changes: 16 additions & 2 deletions pkg/providers/vsphere/vmlifecycle/update_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func ReconcileStatus(
errs = append(errs, reconcileStatusAnno2Conditions(vmCtx, k8sClient, vcVM, data)...)
errs = append(errs, reconcileStatusClass(vmCtx, k8sClient, vcVM, data)...)
errs = append(errs, reconcileStatusPowerState(vmCtx, k8sClient, vcVM, data)...)
errs = append(errs, reconcileStatusIdentifiers(vmCtx, k8sClient, vcVM, data)...)
errs = append(errs, reconcileStatusPlatform(vmCtx, k8sClient, vcVM, data)...)
errs = append(errs, reconcileStatusHardware(vmCtx, k8sClient, vcVM, data)...)
errs = append(errs, reconcileStatusHardwareVersion(vmCtx, k8sClient, vcVM, data)...)
errs = append(errs, reconcileStatusGuest(vmCtx, k8sClient, vcVM, data)...)
Expand Down Expand Up @@ -327,16 +327,30 @@ func reconcileStatusPowerState(
return nil
}

func reconcileStatusIdentifiers(
// reconcileStatusPlatform updates the VM's status with immutable metadata
// from vCenter, including identifiers (UniqueID, BiosUUID, InstanceUUID)
// and provider-specific information like the creation timestamp.
func reconcileStatusPlatform(
vmCtx pkgctx.VirtualMachineContext,
_ ctrlclient.Client,
_ *object.VirtualMachine,
_ ReconcileStatusData) []error { //nolint:unparam

// TODO(v1alpha6): Deprecate and migrate these fields to the Provider status.
vmCtx.VM.Status.UniqueID = vmCtx.MoVM.Self.Value
vmCtx.VM.Status.BiosUUID = vmCtx.MoVM.Summary.Config.Uuid
vmCtx.VM.Status.InstanceUUID = vmCtx.MoVM.Summary.Config.InstanceUuid

if vmCtx.VM.Status.Provider == nil {
vmCtx.VM.Status.Provider = &vmopv1.VirtualMachineProviderStatus{}
}
// The creation timestamp is immutable once set.
if vmCtx.VM.Status.Provider.CreationTimestamp == nil {
if config := vmCtx.MoVM.Config; config != nil && config.CreateDate != nil {
vmCtx.VM.Status.Provider.CreationTimestamp = &metav1.Time{Time: *config.CreateDate}
}
}

return nil
}

Expand Down
69 changes: 69 additions & 0 deletions pkg/providers/vsphere/vmlifecycle/update_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3607,6 +3607,75 @@ var _ = Describe("UpdateStatus", func() {
Expect(status.InstanceUUID).To(Equal(instanceUUID))
Expect(status.HardwareVersion).To(Equal(int32(19)))
})

Context("Provider.CreationTimestamp", func() {
var createDate *metav1.Time

BeforeEach(func() {
t := metav1.Now()
createDate = &t
if vmCtx.MoVM.Config == nil {
vmCtx.MoVM.Config = &vimtypes.VirtualMachineConfigInfo{}
}
vmCtx.MoVM.Config.CreateDate = &createDate.Time
})

When("Provider.CreationTimestamp is not set in status", func() {
BeforeEach(func() {
vmCtx.VM.Status.Provider = nil
})

It("should set Provider.CreationTimestamp from config.createDate", func() {
Expect(vmCtx.VM.Status.Provider).ToNot(BeNil())
Expect(vmCtx.VM.Status.Provider.CreationTimestamp).ToNot(BeNil())
Expect(vmCtx.VM.Status.Provider.CreationTimestamp.Time).To(Equal(createDate.Time))
})
})

When("Provider.CreationTimestamp is already set in status", func() {
var existingDate *metav1.Time

BeforeEach(func() {
// set created at to 1 day ago.
t := metav1.NewTime(metav1.Now().Add(-24 * 3600 * 1000000000))
existingDate = &t
vmCtx.VM.Status.Provider = &vmopv1.VirtualMachineProviderStatus{
CreationTimestamp: existingDate,
}
})

It("should not change Provider.CreationTimestamp because it is immutable", func() {
Expect(vmCtx.VM.Status.Provider).ToNot(BeNil())
Expect(vmCtx.VM.Status.Provider.CreationTimestamp).ToNot(BeNil())
Expect(vmCtx.VM.Status.Provider.CreationTimestamp.Time).To(Equal(existingDate.Time))
Expect(vmCtx.VM.Status.Provider.CreationTimestamp.Time).ToNot(Equal(createDate.Time))
})
})

When("config.createDate is nil", func() {
BeforeEach(func() {
vmCtx.MoVM.Config.CreateDate = nil
vmCtx.VM.Status.Provider = nil
})

It("should initialize Provider but not set CreationTimestamp", func() {
Expect(vmCtx.VM.Status.Provider).ToNot(BeNil())
Expect(vmCtx.VM.Status.Provider.CreationTimestamp).To(BeNil())
})
})

When("config is nil", func() {
BeforeEach(func() {
vmCtx.MoVM.Config = nil
vmCtx.VM.Status.Provider = nil
})

It("should initialize Provider but not set CreationTimestamp", func() {
Expect(vmCtx.VM.Status.Provider).ToNot(BeNil())
Expect(vmCtx.VM.Status.Provider.CreationTimestamp).To(BeNil())
})
})
})
})

Context("Misc Status fields", func() {
Expand Down