diff --git a/pkg/apis/kops/v1alpha2/defaults.go b/pkg/apis/kops/v1alpha2/defaults.go index 256e0bd157..4d97f424b7 100644 --- a/pkg/apis/kops/v1alpha2/defaults.go +++ b/pkg/apis/kops/v1alpha2/defaults.go @@ -46,25 +46,28 @@ func SetDefaults_ClusterSpec(obj *ClusterSpec) { obj.Topology.DNS.Type = DNSTypePublic } - if obj.API == nil { - obj.API = &AccessSpec{} - } - - if obj.API.IsEmpty() { - switch obj.Topology.Masters { - case TopologyPublic: - obj.API.DNS = &DNSAccessSpec{} - - case TopologyPrivate: - obj.API.LoadBalancer = &LoadBalancerAccessSpec{} - - default: - klog.Infof("unknown master topology type: %q", obj.Topology.Masters) + if obj.CloudProvider != "openstack" { + if obj.API == nil { + obj.API = &AccessSpec{} + } + + if obj.API.IsEmpty() { + switch obj.Topology.Masters { + case TopologyPublic: + obj.API.DNS = &DNSAccessSpec{} + + case TopologyPrivate: + obj.API.LoadBalancer = &LoadBalancerAccessSpec{} + + default: + klog.Infof("unknown master topology type: %q", obj.Topology.Masters) + } + } + + if obj.API.LoadBalancer != nil && obj.API.LoadBalancer.Type == "" { + obj.API.LoadBalancer.Type = LoadBalancerTypePublic } - } - if obj.API.LoadBalancer != nil && obj.API.LoadBalancer.Type == "" { - obj.API.LoadBalancer.Type = LoadBalancerTypePublic } if obj.Authorization == nil { diff --git a/pkg/model/openstackmodel/firewall.go b/pkg/model/openstackmodel/firewall.go index 7830222b64..2a2a36d2d9 100644 --- a/pkg/model/openstackmodel/firewall.go +++ b/pkg/model/openstackmodel/firewall.go @@ -479,18 +479,16 @@ func (b *FirewallModelBuilder) Build(c *fi.ModelBuilderContext) error { if b.UseLoadBalancerForAPI() && b.UseVIPACL() { useVIPACL = true } - if b.UseLoadBalancerForAPI() { - sg := &openstacktasks.SecurityGroup{ - Name: s(b.Cluster.Spec.MasterPublicName), - Lifecycle: b.Lifecycle, - RemoveExtraRules: []string{"port=443"}, - } - if useVIPACL { - sg.RemoveGroup = true - } - c.AddTask(sg) - sgMap[b.Cluster.Spec.MasterPublicName] = sg + sg := &openstacktasks.SecurityGroup{ + Name: s(b.Cluster.Spec.MasterPublicName), + Lifecycle: b.Lifecycle, + RemoveExtraRules: []string{"port=443"}, } + if useVIPACL { + sg.RemoveGroup = true + } + c.AddTask(sg) + sgMap[b.Cluster.Spec.MasterPublicName] = sg for _, role := range roles { // Create Security Group for Role diff --git a/pkg/model/openstackmodel/servergroup.go b/pkg/model/openstackmodel/servergroup.go index f066edaba6..a6087b4cc2 100644 --- a/pkg/model/openstackmodel/servergroup.go +++ b/pkg/model/openstackmodel/servergroup.go @@ -159,28 +159,28 @@ func (b *ServerGroupModelBuilder) buildInstances(c *fi.ModelBuilderContext, sg * case kops.InstanceGroupRoleBastion: t := &openstacktasks.FloatingIP{ Name: fi.String(fmt.Sprintf("%s-%s", "fip", *instanceTask.Name)), - Server: instanceTask, Lifecycle: b.Lifecycle, } c.AddTask(t) + instanceTask.FloatingIP = t case kops.InstanceGroupRoleMaster: if b.Cluster.Spec.CloudConfig.Openstack.Loadbalancer == nil { t := &openstacktasks.FloatingIP{ Name: fi.String(fmt.Sprintf("%s-%s", "fip", *instanceTask.Name)), - Server: instanceTask, Lifecycle: b.Lifecycle, } c.AddTask(t) b.associateFIPToKeypair(t) + instanceTask.FloatingIP = t } default: - if !b.UsesSSHBastion() { + if b.Cluster.Spec.Topology == nil || b.Cluster.Spec.Topology.Nodes == "public" { t := &openstacktasks.FloatingIP{ Name: fi.String(fmt.Sprintf("%s-%s", "fip", *instanceTask.Name)), - Server: instanceTask, Lifecycle: b.Lifecycle, } c.AddTask(t) + instanceTask.FloatingIP = t } } } else if b.Cluster.Spec.CloudConfig.Openstack != nil && b.Cluster.Spec.CloudConfig.Openstack.Router == nil { diff --git a/pkg/model/openstackmodel/servergroup_test.go b/pkg/model/openstackmodel/servergroup_test.go index dfe711fee3..9dda980922 100644 --- a/pkg/model/openstackmodel/servergroup_test.go +++ b/pkg/model/openstackmodel/servergroup_test.go @@ -140,7 +140,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } masterFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-master-1-cluster"), - Server: masterInstance, Lifecycle: &clusterLifecycle, } nodeServerGroup := &openstacktasks.ServerGroup{ @@ -186,7 +185,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } nodeFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-node-1-cluster"), - Server: nodeInstance, Lifecycle: &clusterLifecycle, } return map[string]fi.Task{ @@ -226,6 +224,9 @@ func Test_ServerGroupModelBuilder(t *testing.T) { Region: "region", }, }, + Topology: &kops.TopologySpec{ + Nodes: "private", + }, }, }, instanceGroups: []*kops.InstanceGroup{ @@ -325,7 +326,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } masterFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-master-1-cluster"), - Server: masterInstance, Lifecycle: &clusterLifecycle, } nodeServerGroup := &openstacktasks.ServerGroup{ @@ -411,7 +411,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } bastionFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-bastion-1-cluster"), - Server: bastionInstance, Lifecycle: &clusterLifecycle, } return map[string]fi.Task{ @@ -592,7 +591,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } masterAFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-master-a-1-cluster"), - Server: masterAInstance, Lifecycle: &clusterLifecycle, } masterBServerGroup := &openstacktasks.ServerGroup{ @@ -639,7 +637,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } masterBFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-master-b-1-cluster"), - Server: masterBInstance, Lifecycle: &clusterLifecycle, } masterCServerGroup := &openstacktasks.ServerGroup{ @@ -686,7 +683,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } masterCFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-master-c-1-cluster"), - Server: masterCInstance, Lifecycle: &clusterLifecycle, } nodeAServerGroup := &openstacktasks.ServerGroup{ @@ -732,7 +728,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } nodeAFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-node-a-1-cluster"), - Server: nodeAInstance, Lifecycle: &clusterLifecycle, } nodeBServerGroup := &openstacktasks.ServerGroup{ @@ -778,7 +773,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } nodeBFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-node-b-1-cluster"), - Server: nodeBInstance, Lifecycle: &clusterLifecycle, } nodeCServerGroup := &openstacktasks.ServerGroup{ @@ -824,7 +818,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } nodeCFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-node-c-1-cluster"), - Server: nodeCInstance, Lifecycle: &clusterLifecycle, } return map[string]fi.Task{ @@ -1144,7 +1137,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } nodeAFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-node-a-1-cluster"), - Server: nodeAInstance, Lifecycle: &clusterLifecycle, } nodeBServerGroup := &openstacktasks.ServerGroup{ @@ -1190,7 +1182,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } nodeBFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-node-b-1-cluster"), - Server: nodeBInstance, Lifecycle: &clusterLifecycle, } nodeCServerGroup := &openstacktasks.ServerGroup{ @@ -1236,7 +1227,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } nodeCFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-node-c-1-cluster"), - Server: nodeCInstance, Lifecycle: &clusterLifecycle, } loadbalancer := &openstacktasks.LB{ @@ -1827,7 +1817,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } masterAFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-master-1-cluster"), - Server: masterAInstance, Lifecycle: &clusterLifecycle, } masterBPort := &openstacktasks.Port{ @@ -1866,7 +1855,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } masterBFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-master-2-cluster"), - Server: masterBInstance, Lifecycle: &clusterLifecycle, } masterCPort := &openstacktasks.Port{ @@ -1905,7 +1893,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } masterCFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-master-3-cluster"), - Server: masterCInstance, Lifecycle: &clusterLifecycle, } nodeServerGroup := &openstacktasks.ServerGroup{ @@ -1951,7 +1938,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } nodeAFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-node-1-cluster"), - Server: nodeAInstance, Lifecycle: &clusterLifecycle, } nodeBPort := &openstacktasks.Port{ @@ -1989,7 +1975,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } nodeBFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-node-2-cluster"), - Server: nodeBInstance, Lifecycle: &clusterLifecycle, } nodeCPort := &openstacktasks.Port{ @@ -2027,7 +2012,6 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } nodeCFloatingIP := &openstacktasks.FloatingIP{ Name: s("fip-node-3-cluster"), - Server: nodeCInstance, Lifecycle: &clusterLifecycle, } return map[string]fi.Task{ @@ -3040,11 +3024,7 @@ func compareFloatingIPs(t *testing.T, actualTask fi.Task, expected *openstacktas compareStrings(t, "Name", actual.Name, expected.Name) compareLifecycles(t, actual.Lifecycle, expected.Lifecycle) - if pointersAreBothNil(t, "Server", actual.Server, expected.Server) { - compareLoadbalancers(t, actual.LB, expected.LB) - } else { - compareInstances(t, actual.Server, expected.Server) - } + compareLoadbalancers(t, actual.LB, expected.LB) } func compareLifecycles(t *testing.T, actual, expected *fi.Lifecycle) { diff --git a/upup/pkg/fi/cloudup/openstack/cloud.go b/upup/pkg/fi/cloudup/openstack/cloud.go index 3fee96f5b7..d59c8e42d7 100644 --- a/upup/pkg/fi/cloudup/openstack/cloud.go +++ b/upup/pkg/fi/cloudup/openstack/cloud.go @@ -83,7 +83,7 @@ var readBackoff = wait.Backoff{ // writeBackoff is the backoff strategy for openstack write retries. var writeBackoff = wait.Backoff{ Duration: time.Second, - Factor: 1.5, + Factor: 2, Jitter: 0.1, Steps: 5, } @@ -296,7 +296,6 @@ type OpenstackCloud interface { ListFloatingIPs() (fips []floatingips.FloatingIP, err error) ListL3FloatingIPs(opts l3floatingip.ListOpts) (fips []l3floatingip.FloatingIP, err error) - CreateFloatingIP(opts floatingips.CreateOpts) (*floatingips.FloatingIP, error) CreateL3FloatingIP(opts l3floatingip.CreateOpts) (fip *l3floatingip.FloatingIP, err error) DeleteFloatingIP(id string) error DeleteL3FloatingIP(id string) error diff --git a/upup/pkg/fi/cloudup/openstack/floatingip.go b/upup/pkg/fi/cloudup/openstack/floatingip.go index 791560c253..bf5869ac87 100644 --- a/upup/pkg/fi/cloudup/openstack/floatingip.go +++ b/upup/pkg/fi/cloudup/openstack/floatingip.go @@ -43,24 +43,6 @@ func (c *openstackCloud) GetFloatingIP(id string) (fip *floatingips.FloatingIP, return fip, nil } -func (c *openstackCloud) CreateFloatingIP(opts floatingips.CreateOpts) (fip *floatingips.FloatingIP, err error) { - done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) { - - fip, err = floatingips.Create(c.ComputeClient(), opts).Extract() - if err != nil { - return false, fmt.Errorf("CreateFloatingIP: create floating IP failed: %v", err) - } - return true, nil - }) - if !done { - if err == nil { - err = wait.ErrWaitTimeout - } - return fip, err - } - return fip, nil -} - func (c *openstackCloud) AssociateFloatingIPToInstance(serverID string, opts floatingips.AssociateOpts) (err error) { done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) { err = floatingips.AssociateInstance(c.ComputeClient(), serverID, opts).ExtractErr() diff --git a/upup/pkg/fi/cloudup/openstacktasks/floatingip.go b/upup/pkg/fi/cloudup/openstacktasks/floatingip.go index 48689db404..2e33aaa0f2 100644 --- a/upup/pkg/fi/cloudup/openstacktasks/floatingip.go +++ b/upup/pkg/fi/cloudup/openstacktasks/floatingip.go @@ -20,7 +20,6 @@ import ( "fmt" "time" - "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/floatingips" l3floatingip "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/floatingips" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/klog" @@ -33,8 +32,8 @@ import ( type FloatingIP struct { Name *string ID *string - Server *Instance LB *LB + IP *string Lifecycle *fi.Lifecycle ForAPIServer bool } @@ -71,27 +70,12 @@ func findL3Floating(cloud openstack.OpenstackCloud, opts l3floatingip.ListOpts) return result, nil } -func (e *FloatingIP) findServerFloatingIP(context *fi.Context, cloud openstack.OpenstackCloud) (*string, error) { - ips, err := cloud.ListServerFloatingIPs(fi.StringValue(e.Server.ID)) - if err != nil { - return nil, err - } - // assumes that we do have only one interface and 0-1 floatingip in server, the setup that kops does - if len(ips) > 0 { - return ips[0], nil - } - return nil, nil -} - func (e *FloatingIP) IsForAPIServer() bool { return e.ForAPIServer } func (e *FloatingIP) FindIPAddress(context *fi.Context) (*string, error) { if e.ID == nil { - if e.Server != nil && e.Server.ID == nil { - return nil, nil - } if e.LB != nil && e.LB.ID == nil { return nil, nil } @@ -110,12 +94,6 @@ func (e *FloatingIP) FindIPAddress(context *fi.Context) (*string, error) { return &fips[0].FloatingIP, nil } return nil, fmt.Errorf("Could not find port floatingips port=%s", fi.StringValue(e.LB.PortID)) - } else if e.ID == nil && e.Server != nil { - floating, err := e.findServerFloatingIP(context, cloud) - if err != nil { - return nil, fmt.Errorf("Could not find server (\"%s\") floating ip: %v", fi.StringValue(e.Server.ID), err) - } - return floating, nil } fip, err := cloud.GetFloatingIP(fi.StringValue(e.ID)) @@ -129,9 +107,6 @@ func (e *FloatingIP) FindIPAddress(context *fi.Context) (*string, error) { func (e *FloatingIP) GetDependencies(tasks map[string]fi.Task) []fi.Task { var deps []fi.Task for _, task := range tasks { - if _, ok := task.(*Instance); ok { - deps = append(deps, task) - } if _, ok := task.(*LB); ok { deps = append(deps, task) } @@ -177,34 +152,25 @@ func (e *FloatingIP) Find(c *fi.Context) (*FloatingIP, error) { } e.ID = actual.ID return actual, nil - } else if e.Server != nil { - fips, err := cloud.ListFloatingIPs() - if err != nil { - return nil, fmt.Errorf("Failed to list floating ip's: %v", err) - } - if len(fips) == 0 { - return nil, nil - } - var actual *FloatingIP - for i, fip := range fips { - if fip.InstanceID == fi.StringValue(e.Server.ID) { - if actual != nil { - return nil, fmt.Errorf("Multiple floating ip's associated to server: %s", fi.StringValue(e.Server.ID)) - } - actual = &FloatingIP{ - Name: e.Name, - ID: fi.String(fips[i].ID), - Server: e.Server, - Lifecycle: e.Lifecycle, - } - break - } - } - if actual != nil { - e.ID = actual.ID - } - return actual, nil } + fips, err := cloud.ListL3FloatingIPs(l3floatingip.ListOpts{}) + if err != nil { + return nil, fmt.Errorf("failed to list layer 3 floating ip's: %v", err) + } + for _, fip := range fips { + if fip.Description == fi.StringValue(e.Name) { + actual := &FloatingIP{ + ID: fi.String(fips[0].ID), + Name: e.Name, + IP: fi.String(fip.FloatingIP), + Lifecycle: e.Lifecycle, + } + e.ID = actual.ID + e.IP = actual.IP + return actual, nil + } + } + return nil, nil } @@ -262,43 +228,18 @@ func (f *FloatingIP) RenderOpenstack(t *openstack.OpenstackAPITarget, a, e, chan e.ID = fi.String(fip.ID) - } else if e.Server != nil { - - if err := e.Server.WaitForStatusActive(t); err != nil { - return fmt.Errorf("Failed to associate floating IP to instance %s", *e.Name) - } - - // recheck is there floatingip already in port - // this can happen for instance when recreating bastion host - fips, err := cloud.ListFloatingIPs() - if err != nil { - return fmt.Errorf("Failed to list floating ip's: %v", err) - } - for _, fip := range fips { - if fip.InstanceID == fi.StringValue(e.Server.ID) { - e.ID = fi.String(fip.ID) - klog.V(2).Infof("Openstack::RenderOpenstack floatingip found after server is active") - return nil - } - } - - fip, err := cloud.CreateFloatingIP(floatingips.CreateOpts{ - Pool: external.Name, - }) - if err != nil { - return fmt.Errorf("Failed to create floating IP: %v", err) - } - err = cloud.AssociateFloatingIPToInstance(fi.StringValue(e.Server.ID), floatingips.AssociateOpts{ - FloatingIP: fip.IP, - }) - if err != nil { - return fmt.Errorf("Failed to associated floating IP to instance %s: %v", *e.Name, err) - } - - e.ID = fi.String(fip.ID) - } else { - return fmt.Errorf("Must specify either Port or Server!") + + fip, err := cloud.CreateL3FloatingIP(l3floatingip.CreateOpts{ + FloatingNetworkID: external.ID, + Description: fi.StringValue(e.Name), + }) + if err != nil { + return fmt.Errorf("failed to create floating IP: %v", err) + } + e.ID = fi.String(fip.ID) + e.IP = fi.String(fip.FloatingIP) + } return nil } diff --git a/upup/pkg/fi/cloudup/openstacktasks/instance.go b/upup/pkg/fi/cloudup/openstacktasks/instance.go index 44ff7ee0c2..60c1d32088 100644 --- a/upup/pkg/fi/cloudup/openstacktasks/instance.go +++ b/upup/pkg/fi/cloudup/openstacktasks/instance.go @@ -21,6 +21,7 @@ import ( "strconv" "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/bootfromvolume" + "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/floatingips" "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/keypairs" "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/schedulerhints" "github.com/gophercloud/gophercloud/openstack/compute/v2/servers" @@ -45,6 +46,7 @@ type Instance struct { Metadata map[string]string AvailabilityZone *string SecurityGroups []string + FloatingIP *FloatingIP Lifecycle *fi.Lifecycle ForAPIServer bool @@ -64,6 +66,9 @@ func (e *Instance) GetDependencies(tasks map[string]fi.Task) []fi.Task { if _, ok := task.(*Port); ok { deps = append(deps, task) } + if _, ok := task.(*FloatingIP); ok { + deps = append(deps, task) + } } if e.UserData != nil { @@ -109,7 +114,8 @@ func (e *Instance) Find(c *fi.Context) (*Instance, error) { if e == nil || e.Name == nil { return nil, nil } - serverPage, err := servers.List(c.Cloud.(openstack.OpenstackCloud).ComputeClient(), servers.ListOpts{ + client := c.Cloud.(openstack.OpenstackCloud).ComputeClient() + serverPage, err := servers.List(client, servers.ListOpts{ Name: fmt.Sprintf("^%s$", fi.StringValue(e.Name)), }).AllPages() if err != nil { @@ -160,7 +166,11 @@ func (_ *Instance) CheckChanges(a, e, changes *Instance) error { } func (_ *Instance) ShouldCreate(a, e, changes *Instance) (bool, error) { - return a == nil, nil + if a == nil { + return true, nil + } + + return false, nil } func (_ *Instance) RenderOpenstack(t *openstack.OpenstackAPITarget, a, e, changes *Instance) error { @@ -214,12 +224,29 @@ func (_ *Instance) RenderOpenstack(t *openstack.OpenstackAPITarget, a, e, change e.ID = fi.String(v.ID) e.ServerGroup.Members = append(e.ServerGroup.Members, fi.StringValue(e.ID)) + if e.FloatingIP != nil { + err = associateFloatingIP(t, e) + } + if err != nil { + return err + } + klog.V(2).Infof("Creating a new Openstack instance, id=%s", v.ID) - return nil } - klog.V(2).Infof("Openstack task Instance::RenderOpenstack did nothing") + return nil +} + +func associateFloatingIP(t *openstack.OpenstackAPITarget, e *Instance) error { + cloud := t.Cloud.(openstack.OpenstackCloud) + + err := cloud.AssociateFloatingIPToInstance(fi.StringValue(e.ID), floatingips.AssociateOpts{ + FloatingIP: fi.StringValue(e.FloatingIP.IP), + }) + if err != nil { + return fmt.Errorf("failed to associated floating IP to instance %s: %v", *e.Name, err) + } return nil }