Skip to content

Commit

Permalink
Resolved and Resources become pointers in machine status
Browse files Browse the repository at this point in the history
  • Loading branch information
mdbooth committed Mar 25, 2024
1 parent 59689c9 commit 2758a6d
Show file tree
Hide file tree
Showing 16 changed files with 207 additions and 105 deletions.
2 changes: 1 addition & 1 deletion api/v1alpha5/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func TestConvertFrom(t *testing.T) {
Spec: OpenStackMachineSpec{},
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"cluster.x-k8s.io/conversion-data": "{\"spec\":{\"flavor\":\"\",\"image\":{}},\"status\":{\"ready\":false,\"resolved\":{},\"resources\":{}}}",
"cluster.x-k8s.io/conversion-data": "{\"spec\":{\"flavor\":\"\",\"image\":{}},\"status\":{\"ready\":false}}",
},
},
},
Expand Down
76 changes: 42 additions & 34 deletions api/v1alpha6/openstackcluster_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,12 +367,7 @@ func restorev1beta1ClusterStatus(previous *infrav1.OpenStackClusterStatus, dst *
dst.WorkerSecurityGroup = previous.WorkerSecurityGroup
dst.BastionSecurityGroup = previous.BastionSecurityGroup

if previous.Bastion != nil {
dst.Bastion.Resolved = previous.Bastion.Resolved
}
if previous.Bastion != nil && previous.Bastion.Resources.Ports != nil {
dst.Bastion.Resources.Ports = previous.Bastion.Resources.Ports
}
restorev1beta1BastionStatus(previous.Bastion, dst.Bastion)
}

func Convert_v1beta1_OpenStackClusterStatus_To_v1alpha6_OpenStackClusterStatus(in *infrav1.OpenStackClusterStatus, out *OpenStackClusterStatus, s apiconversion.Scope) error {
Expand Down Expand Up @@ -412,36 +407,15 @@ func Convert_v1alpha6_OpenStackClusterStatus_To_v1beta1_OpenStackClusterStatus(i
/* Bastion */

func restorev1beta1Bastion(previous **infrav1.Bastion, dst **infrav1.Bastion) {
if *previous != nil {
if *dst != nil && (*previous).Spec != nil && (*dst).Spec != nil {
restorev1beta1MachineSpec((*previous).Spec, (*dst).Spec)
}

optional.RestoreString(&(*previous).FloatingIP, &(*dst).FloatingIP)
optional.RestoreString(&(*previous).AvailabilityZone, &(*dst).AvailabilityZone)
if previous == nil || dst == nil || *previous == nil || *dst == nil {
return
}
if *dst != nil && (*previous).Spec != nil && (*dst).Spec != nil {
restorev1beta1MachineSpec((*previous).Spec, (*dst).Spec)
}
}

func Convert_v1alpha6_Instance_To_v1beta1_BastionStatus(in *Instance, out *infrav1.BastionStatus, _ apiconversion.Scope) error {
// BastionStatus is the same as Spec with unused fields removed
out.ID = in.ID
out.Name = in.Name
out.SSHKeyName = in.SSHKeyName
out.State = infrav1.InstanceState(in.State)
out.IP = in.IP
out.FloatingIP = in.FloatingIP
return nil
}

func Convert_v1beta1_BastionStatus_To_v1alpha6_Instance(in *infrav1.BastionStatus, out *Instance, _ apiconversion.Scope) error {
// BastionStatus is the same as Spec with unused fields removed
out.ID = in.ID
out.Name = in.Name
out.SSHKeyName = in.SSHKeyName
out.State = InstanceState(in.State)
out.IP = in.IP
out.FloatingIP = in.FloatingIP
return nil
optional.RestoreString(&(*previous).FloatingIP, &(*dst).FloatingIP)
optional.RestoreString(&(*previous).AvailabilityZone, &(*dst).AvailabilityZone)
}

func Convert_v1alpha6_Bastion_To_v1beta1_Bastion(in *Bastion, out *infrav1.Bastion, s apiconversion.Scope) error {
Expand Down Expand Up @@ -496,3 +470,37 @@ func Convert_v1beta1_Bastion_To_v1alpha6_Bastion(in *infrav1.Bastion, out *Basti

return optional.Convert_optional_String_To_string(&in.FloatingIP, &out.Instance.FloatingIP, s)
}

/* Bastion status */

func restorev1beta1BastionStatus(previous *infrav1.BastionStatus, dst *infrav1.BastionStatus) {
if previous == nil || dst == nil {
return
}

// Resolved and resources have no equivalents
dst.Resolved = previous.Resolved
dst.Resources = previous.Resources
}

func Convert_v1alpha6_Instance_To_v1beta1_BastionStatus(in *Instance, out *infrav1.BastionStatus, _ apiconversion.Scope) error {
// BastionStatus is the same as Spec with unused fields removed
out.ID = in.ID
out.Name = in.Name
out.SSHKeyName = in.SSHKeyName
out.State = infrav1.InstanceState(in.State)
out.IP = in.IP
out.FloatingIP = in.FloatingIP
return nil
}

func Convert_v1beta1_BastionStatus_To_v1alpha6_Instance(in *infrav1.BastionStatus, out *Instance, _ apiconversion.Scope) error {
// BastionStatus is the same as Spec with unused fields removed
out.ID = in.ID
out.Name = in.Name
out.SSHKeyName = in.SSHKeyName
out.State = InstanceState(in.State)
out.IP = in.IP
out.FloatingIP = in.FloatingIP
return nil
}
4 changes: 2 additions & 2 deletions api/v1alpha6/openstackmachine_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,13 @@ var v1beta1OpenStackMachineRestorer = conversion.RestorerFor[*infrav1.OpenStackM
restorev1beta1MachineSpec,
),
"depresources": conversion.UnconditionalFieldRestorer(
func(c *infrav1.OpenStackMachine) *infrav1.MachineResources {
func(c *infrav1.OpenStackMachine) **infrav1.MachineResources {
return &c.Status.Resources
},
),
// No equivalent in v1alpha6
"refresources": conversion.UnconditionalFieldRestorer(
func(c *infrav1.OpenStackMachine) *infrav1.ResolvedMachineSpec {
func(c *infrav1.OpenStackMachine) **infrav1.ResolvedMachineSpec {
return &c.Status.Resolved
},
),
Expand Down
21 changes: 15 additions & 6 deletions api/v1alpha7/openstackcluster_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,14 +348,11 @@ func restorev1alpha7ClusterStatus(previous *OpenStackClusterStatus, dst *OpenSta
}

func restorev1beta1ClusterStatus(previous *infrav1.OpenStackClusterStatus, dst *infrav1.OpenStackClusterStatus) {
// ReferencedResources have no equivalent in v1alpha7
if previous.Bastion != nil {
dst.Bastion.Resolved = previous.Bastion.Resolved
if previous == nil || dst == nil {
return
}

if previous.Bastion != nil && previous.Bastion.Resources.Ports != nil {
dst.Bastion.Resources.Ports = previous.Bastion.Resources.Ports
}
restorev1beta1BastionStatus(previous.Bastion, dst.Bastion)
}

func Convert_v1beta1_OpenStackClusterStatus_To_v1alpha7_OpenStackClusterStatus(in *infrav1.OpenStackClusterStatus, out *OpenStackClusterStatus, s apiconversion.Scope) error {
Expand Down Expand Up @@ -434,6 +431,18 @@ func Convert_v1beta1_Bastion_To_v1alpha7_Bastion(in *infrav1.Bastion, out *Basti
return optional.Convert_optional_String_To_string(&in.FloatingIP, &out.Instance.FloatingIP, s)
}

/* Bastion status */

func restorev1beta1BastionStatus(previous *infrav1.BastionStatus, dst *infrav1.BastionStatus) {
if previous == nil || dst == nil {
return
}

// Resolved and resources have no equivalents
dst.Resolved = previous.Resolved
dst.Resources = previous.Resources
}

func Convert_v1beta1_BastionStatus_To_v1alpha7_BastionStatus(in *infrav1.BastionStatus, out *BastionStatus, s apiconversion.Scope) error {
// ReferencedResources have no equivalent in v1alpha7
return autoConvert_v1beta1_BastionStatus_To_v1alpha7_BastionStatus(in, out, s)
Expand Down
4 changes: 2 additions & 2 deletions api/v1alpha7/openstackmachine_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,13 @@ var v1beta1OpenStackMachineRestorer = conversion.RestorerFor[*infrav1.OpenStackM
restorev1beta1MachineSpec,
),
"depresources": conversion.UnconditionalFieldRestorer(
func(c *infrav1.OpenStackMachine) *infrav1.MachineResources {
func(c *infrav1.OpenStackMachine) **infrav1.MachineResources {
return &c.Status.Resources
},
),
// No equivalent in v1alpha7
"refresources": conversion.UnconditionalFieldRestorer(
func(c *infrav1.OpenStackMachine) *infrav1.ResolvedMachineSpec {
func(c *infrav1.OpenStackMachine) **infrav1.ResolvedMachineSpec {
return &c.Status.Resolved
},
),
Expand Down
6 changes: 4 additions & 2 deletions api/v1beta1/openstackmachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,12 @@ type OpenStackMachineStatus struct {

// Resolved contains parts of the machine spec with all external
// references fully resolved.
Resolved ResolvedMachineSpec `json:"resolved,omitempty"`
// +optional
Resolved *ResolvedMachineSpec `json:"resolved,omitempty"`

// Resources contains references to OpenStack resources created for the machine.
Resources MachineResources `json:"resources,omitempty"`
// +optional
Resources *MachineResources `json:"resources,omitempty"`

FailureReason *errors.MachineStatusError `json:"failureReason,omitempty"`

Expand Down
23 changes: 15 additions & 8 deletions api/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,14 +358,21 @@ type AddressPair struct {
}

type BastionStatus struct {
ID string `json:"id,omitempty"`
Name string `json:"name,omitempty"`
SSHKeyName string `json:"sshKeyName,omitempty"`
State InstanceState `json:"state,omitempty"`
IP string `json:"ip,omitempty"`
FloatingIP string `json:"floatingIP,omitempty"`
Resolved ResolvedMachineSpec `json:"resolved,omitempty"`
Resources MachineResources `json:"resources,omitempty"`
ID string `json:"id,omitempty"`
Name string `json:"name,omitempty"`
SSHKeyName string `json:"sshKeyName,omitempty"`
State InstanceState `json:"state,omitempty"`
IP string `json:"ip,omitempty"`
FloatingIP string `json:"floatingIP,omitempty"`

// Resolved contains parts of the bastion's machine spec with all
// external references fully resolved.
// +optional
Resolved *ResolvedMachineSpec `json:"resolved,omitempty"`

// Resources contains references to OpenStack resources created for the bastion.
// +optional
Resources *MachineResources `json:"resources,omitempty"`
}

type RootVolume struct {
Expand Down
24 changes: 20 additions & 4 deletions api/v1beta1/zz_generated.deepcopy.go

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

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

36 changes: 26 additions & 10 deletions controllers/openstackcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,13 @@ func resolveBastionResources(scope *scope.WithLogger, clusterResourceName string
if openStackCluster.Spec.Bastion.Spec == nil {
return false, fmt.Errorf("bastion spec is nil when bastion is enabled, this shouldn't happen")
}
resolved := openStackCluster.Status.Bastion.Resolved
if resolved == nil {
resolved = &infrav1.ResolvedMachineSpec{}
openStackCluster.Status.Bastion.Resolved = resolved
}
changed, err := compute.ResolveMachineSpec(scope,
openStackCluster.Spec.Bastion.Spec, &openStackCluster.Status.Bastion.Resolved,
openStackCluster.Spec.Bastion.Spec, resolved,
clusterResourceName, bastionName(clusterResourceName),
openStackCluster, getBastionSecurityGroupID(openStackCluster))
if err != nil {
Expand All @@ -238,10 +243,13 @@ func resolveBastionResources(scope *scope.WithLogger, clusterResourceName string
// If the resolved machine spec changed we need to restart the reconcile to avoid inconsistencies between reconciles.
return true, nil
}
resources := openStackCluster.Status.Bastion.Resources
if resources == nil {
resources = &infrav1.MachineResources{}
openStackCluster.Status.Bastion.Resources = resources
}

err = compute.AdoptMachineResources(scope,
&openStackCluster.Status.Bastion.Resolved,
&openStackCluster.Status.Bastion.Resources)
err = compute.AdoptMachineResources(scope, resolved, resources)
if err != nil {
return false, err
}
Expand All @@ -268,8 +276,10 @@ func deleteBastion(scope *scope.WithLogger, cluster *clusterv1.Cluster, openStac
}
}

bastionStatus := openStackCluster.Status.Bastion

var instanceStatus *compute.InstanceStatus
if openStackCluster.Status.Bastion != nil && openStackCluster.Status.Bastion.ID != "" {
if bastionStatus != nil && bastionStatus.ID != "" {
instanceStatus, err = computeService.GetInstanceStatus(openStackCluster.Status.Bastion.ID)
if err != nil {
return err
Expand Down Expand Up @@ -308,18 +318,18 @@ func deleteBastion(scope *scope.WithLogger, cluster *clusterv1.Cluster, openStac
}
}

if openStackCluster.Status.Bastion != nil && len(openStackCluster.Status.Bastion.Resources.Ports) > 0 {
if bastionStatus != nil && bastionStatus.Resources != nil {
trunkSupported, err := networkingService.IsTrunkExtSupported()
if err != nil {
return err
}
for _, port := range openStackCluster.Status.Bastion.Resources.Ports {
for _, port := range bastionStatus.Resources.Ports {
if err := networkingService.DeleteInstanceTrunkAndPort(openStackCluster, port, trunkSupported); err != nil {
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to delete port: %w", err))
return fmt.Errorf("failed to delete port: %w", err)
}
}
openStackCluster.Status.Bastion.Resources.Ports = nil
bastionStatus.Resources.Ports = nil
}

scope.Logger().Info("Deleted Bastion")
Expand Down Expand Up @@ -542,7 +552,10 @@ func bastionToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, cluster *
// v1beta1 API validations prevent this from happening in normal circumstances.
bastion.Spec = &infrav1.OpenStackMachineSpec{}
}
resolved := &openStackCluster.Status.Bastion.Resolved
resolved := openStackCluster.Status.Bastion.Resolved
if resolved == nil {
return nil, errors.New("bastion resolved is nil")
}

machineSpec := bastion.Spec
instanceSpec := &compute.InstanceSpec{
Expand Down Expand Up @@ -579,7 +592,10 @@ func getBastionSecurityGroupID(openStackCluster *infrav1.OpenStackCluster) *stri

func getOrCreateBastionPorts(openStackCluster *infrav1.OpenStackCluster, networkingService *networking.Service) error {
desiredPorts := openStackCluster.Status.Bastion.Resolved.Ports
resources := &openStackCluster.Status.Bastion.Resources
resources := openStackCluster.Status.Bastion.Resources
if resources == nil {
return errors.New("bastion resources are nil")
}

if len(desiredPorts) == len(resources.Ports) {
return nil
Expand Down
Loading

0 comments on commit 2758a6d

Please sign in to comment.