Merge pull request #15301 from infonova/os-rework-retry-failed-servers

OpenStack: Use task engine to retry failed servers

pkg/model/openstackmodel/tests/servergroup/adds-additional-security-groups.yaml

@@ -71,6 +71,7 @@ ServerGroup:
   Name: cluster-node
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""

pkg/model/openstackmodel/tests/servergroup/adds-cloud-labels-from-ClusterSpec.yaml

@@ -70,6 +70,7 @@ ServerGroup:
   Name: cluster-node
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""

pkg/model/openstackmodel/tests/servergroup/adds-cloud-labels-from-InstanceGroupSpec.yaml

@@ -70,6 +70,7 @@ ServerGroup:
   Name: cluster-node
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""

pkg/model/openstackmodel/tests/servergroup/configures-allowed-address-pairs-with-annotations.yaml

@@ -72,6 +72,7 @@ ServerGroup:
   Name: cluster-node
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""

pkg/model/openstackmodel/tests/servergroup/configures-server-group-affinity-with-annotations.yaml

@@ -69,6 +69,7 @@ ServerGroup:
   Name: cluster-node
   Policies:
   - soft-anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""

pkg/model/openstackmodel/tests/servergroup/multizone-setup-3-masters-3-nodes-without-bastion-auto-zone-distribution.yaml

@@ -129,6 +129,7 @@ ServerGroup:
   Name: cluster-master
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -217,6 +218,7 @@ ServerGroup:
   Name: cluster-master
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -305,6 +307,7 @@ ServerGroup:
   Name: cluster-master
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -384,6 +387,7 @@ ServerGroup:
   Name: cluster-node
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -463,6 +467,7 @@ ServerGroup:
   Name: cluster-node
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -542,6 +547,7 @@ ServerGroup:
   Name: cluster-node
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""

pkg/model/openstackmodel/tests/servergroup/multizone-setup-3-masters-3-nodes-without-bastion-with-API-loadbalancer-dns-none.yaml

@@ -109,6 +109,7 @@ ServerGroup:
   Name: cluster-master-a
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -185,6 +186,7 @@ ServerGroup:
   Name: cluster-master-b
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -261,6 +263,7 @@ ServerGroup:
   Name: cluster-master-c
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -334,6 +337,7 @@ ServerGroup:
   Name: cluster-node-a
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -407,6 +411,7 @@ ServerGroup:
   Name: cluster-node-b
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -480,6 +485,7 @@ ServerGroup:
   Name: cluster-node-c
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""

pkg/model/openstackmodel/tests/servergroup/multizone-setup-3-masters-3-nodes-without-bastion-with-API-loadbalancer.yaml

@@ -109,6 +109,7 @@ ServerGroup:
   Name: cluster-master-a
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -185,6 +186,7 @@ ServerGroup:
   Name: cluster-master-b
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -261,6 +263,7 @@ ServerGroup:
   Name: cluster-master-c
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -334,6 +337,7 @@ ServerGroup:
   Name: cluster-node-a
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -407,6 +411,7 @@ ServerGroup:
   Name: cluster-node-b
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -480,6 +485,7 @@ ServerGroup:
   Name: cluster-node-c
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""

pkg/model/openstackmodel/tests/servergroup/multizone-setup-3-masters-3-nodes-without-bastion.yaml

@@ -141,6 +141,7 @@ ServerGroup:
   Name: cluster-master-a
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -229,6 +230,7 @@ ServerGroup:
   Name: cluster-master-b
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -317,6 +319,7 @@ ServerGroup:
   Name: cluster-master-c
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -396,6 +399,7 @@ ServerGroup:
   Name: cluster-node-a
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -475,6 +479,7 @@ ServerGroup:
   Name: cluster-node-b
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -554,6 +559,7 @@ ServerGroup:
   Name: cluster-node-c
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""

pkg/model/openstackmodel/tests/servergroup/multizone-setup-3-masters-3-nodes-without-external-router.yaml

@@ -93,6 +93,7 @@ ServerGroup:
   Name: cluster-master-a
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -175,6 +176,7 @@ ServerGroup:
   Name: cluster-master-b
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -257,6 +259,7 @@ ServerGroup:
   Name: cluster-master-c
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -330,6 +333,7 @@ ServerGroup:
   Name: cluster-node-a
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -403,6 +407,7 @@ ServerGroup:
   Name: cluster-node-b
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -476,6 +481,7 @@ ServerGroup:
   Name: cluster-node-c
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""

pkg/model/openstackmodel/tests/servergroup/one-master-one-node-one-bastion-2.yaml

@@ -74,6 +74,7 @@ ServerGroup:
   Name: cluster-bastion
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -156,6 +157,7 @@ ServerGroup:
   Name: cluster-master
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -229,6 +231,7 @@ ServerGroup:
   Name: cluster-node
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""

pkg/model/openstackmodel/tests/servergroup/one-master-one-node-one-bastion.yaml

@@ -87,6 +87,7 @@ ServerGroup:
   Name: cluster-bastion
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -169,6 +170,7 @@ ServerGroup:
   Name: cluster-master
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -242,6 +244,7 @@ ServerGroup:
   Name: cluster-node
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""

pkg/model/openstackmodel/tests/servergroup/one-master-one-node-without-bastion-no-public-ip-association.yaml

@@ -81,6 +81,7 @@ ServerGroup:
   Name: cluster-master
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -154,6 +155,7 @@ ServerGroup:
   Name: cluster-node
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""

pkg/model/openstackmodel/tests/servergroup/one-master-one-node.yaml

@@ -101,6 +101,7 @@ ServerGroup:
   Name: cluster-master
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -180,6 +181,7 @@ ServerGroup:
   Name: cluster-node
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""

pkg/model/openstackmodel/tests/servergroup/truncate-cluster-names-to-42-characters.yaml

@@ -101,6 +101,7 @@ ServerGroup:
   Name: tom-software-dev-playground-real33-k8s-local-master
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""
@@ -180,6 +181,7 @@ ServerGroup:
   Name: tom-software-dev-playground-real33-k8s-local-node
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""

pkg/model/openstackmodel/tests/servergroup/uses-instance-group-subnet-as-availability-zones-fallback.yaml

@@ -71,6 +71,7 @@ ServerGroup:
   Name: cluster-node
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""

pkg/model/openstackmodel/tests/servergroup/uses-instance-group-zones-as-availability-zones.yaml

@@ -71,6 +71,7 @@ ServerGroup:
   Name: cluster-node
   Policies:
   - anti-affinity
+Status: null
 UserData:
   task:
     Lifecycle: ""

upup/pkg/fi/cloudup/openstack/instance.go

@@ -101,10 +101,6 @@ func createInstance(c OpenstackCloud, opt servers.CreateOptsBuilder, portID stri
 	var server *servers.Server
 
 	done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
-		if server != nil {
-			// Note: this will delete the server from the last try, even if it is now ACTIVE or still in BUILD state
-			c.DeleteInstanceWithID(server.ID)
-		}
 
 		v, err := servers.Create(c.ComputeClient(), opt).Extract()
 		if err != nil {
@@ -131,7 +127,7 @@ func createInstance(c OpenstackCloud, opt servers.CreateOptsBuilder, portID stri
 
 		err = waitForStatusActive(c, server.ID, nil)
 		if err != nil {
-			return false, fmt.Errorf("error while waiting for server '%s' to become '%s': %v", server.ID, activeStatus, err)
+			return true, err
 		}
 
 		return true, nil

upup/pkg/fi/cloudup/openstacktasks/instance.go

@@ -52,6 +52,7 @@ type Instance struct {
 	SecurityGroups   []string
 	FloatingIP       *FloatingIP
 	ConfigDrive      *bool
+	Status           *string
 
 	Lifecycle    fi.Lifecycle
 	ForAPIServer bool
@@ -198,6 +199,7 @@ func (e *Instance) Find(c *fi.CloudupContext) (*Instance, error) {
 		AvailabilityZone: e.AvailabilityZone,
 		GroupName:        e.GroupName,
 		ConfigDrive:      e.ConfigDrive,
+		Status:           fi.PtrTo(server.Status),
 	}
 
 	ports, err := cloud.ListPorts(ports.ListOpts{
@@ -244,6 +246,7 @@ func (e *Instance) Find(c *fi.CloudupContext) (*Instance, error) {
 
 	// Avoid flapping
 	e.ID = actual.ID
+	e.Status = fi.PtrTo(activeStatus)
 	actual.ForAPIServer = e.ForAPIServer
 
 	// Immutable fields
@@ -281,6 +284,9 @@ func (_ *Instance) ShouldCreate(a, e, changes *Instance) (bool, error) {
 	if a == nil {
 		return true, nil
 	}
+	if fi.ValueOf(a.Status) == errorStatus {
+		return true, nil
+	}
 	if changes.Port != nil {
 		return true, nil
 	}
@@ -309,7 +315,13 @@ func generateInstanceName(e *Instance) (string, error) {
 
 func (_ *Instance) RenderOpenstack(t *openstack.OpenstackAPITarget, a, e, changes *Instance) error {
 	cloud := t.Cloud
-	if a == nil {
+
+	if a != nil && fi.ValueOf(a.Status) == errorStatus {
+		klog.V(2).Infof("Delete previously failed server: %s\n", fi.ValueOf(a.ID))
+		cloud.DeleteInstanceWithID(fi.ValueOf(a.ID))
+	}
+
+	if a == nil || fi.ValueOf(a.Status) == errorStatus {
 		serverName, err := generateInstanceName(e)
 		if err != nil {
 			return err