Skip to content

Commit

Permalink
Merge pull request kubernetes#16023 from zetaab/automated-cherry-pick…
Browse files Browse the repository at this point in the history
…-of-#16022-upstream-release-1.28

Automated cherry pick of kubernetes#16022: fix instance group validation if using serverGroupName
  • Loading branch information
k8s-ci-robot committed Oct 15, 2023
2 parents 64a4ab3 + 73f6f49 commit 529670c
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 80 deletions.
64 changes: 35 additions & 29 deletions upup/pkg/fi/cloudup/openstack/cloud.go
Expand Up @@ -604,34 +604,56 @@ func (c *openstackCloud) DeleteGroup(g *cloudinstances.CloudInstanceGroup) error
}

func deleteGroup(c OpenstackCloud, g *cloudinstances.CloudInstanceGroup) error {
grp := g.Raw.(*servergroups.ServerGroup)

for _, id := range grp.Members {
err := c.DeleteInstanceWithID(id)
instances, err := c.ListInstances(servers.ListOpts{
Name: fmt.Sprintf("^%s", g.InstanceGroup.Name),
})
if err != nil {
return err
}
for _, instance := range instances {
err := c.DeleteInstanceWithID(instance.ID)
if err != nil {
return fmt.Errorf("could not delete instance %q: %v", id, err)
return fmt.Errorf("could not delete instance %q: %v", instance.ID, err)
}
}

ports, err := c.ListPorts(ports.ListOpts{})
if err != nil {
return fmt.Errorf("Could not list ports %v", err)
return fmt.Errorf("could not list ports %v", err)
}

for _, port := range ports {
if strings.Contains(port.Name, grp.Name) {
if strings.HasPrefix(port.Name, fmt.Sprintf("port-%s", g.InstanceGroup.Name)) {
err := c.DeletePort(port.ID)
if err != nil {
return fmt.Errorf("could not delete port %q: %v", port.ID, err)
}
}
}

err = c.DeleteServerGroup(grp.ID)
sgName := g.InstanceGroup.Name
if name, ok := g.InstanceGroup.Annotations[OS_ANNOTATION+SERVER_GROUP_NAME]; ok {
sgName = name
}
sgs, err := c.ListServerGroups(servergroups.ListOpts{})
if err != nil {
return fmt.Errorf("could not server group %q: %v", grp.ID, err)
return fmt.Errorf("could not list server groups %v", err)
}

cluster := g.Raw.(*kops.Cluster)
for _, sg := range sgs {
if fmt.Sprintf("%s-%s", cluster.Name, sgName) == sg.Name {
if len(sg.Members) == 0 {
err = c.DeleteServerGroup(sg.ID)
if err != nil {
return fmt.Errorf("could not delete server group %q: %v", sg.ID, err)
}
} else {
klog.Infof("Server group %q still has members (IDs: %s), delete not executed", sg.ID, strings.Join(sg.Members, ", "))
}
break
}
}
return nil
}

Expand All @@ -642,27 +664,11 @@ func (c *openstackCloud) GetCloudGroups(cluster *kops.Cluster, instancegroups []
func getCloudGroups(c OpenstackCloud, cluster *kops.Cluster, instancegroups []*kops.InstanceGroup, warnUnmatched bool, nodes []v1.Node) (map[string]*cloudinstances.CloudInstanceGroup, error) {
nodeMap := cloudinstances.GetNodeMap(nodes, cluster)
groups := make(map[string]*cloudinstances.CloudInstanceGroup)

serverGrps, err := c.ListServerGroups(servergroups.ListOpts{})
if err != nil {
return nil, fmt.Errorf("unable to list servergroups: %v", err)
}

for _, grp := range serverGrps {
name := grp.Name
instancegroup, err := matchInstanceGroup(name, cluster.ObjectMeta.Name, instancegroups)
if err != nil {
return nil, fmt.Errorf("error getting instance group for servergroup %q", name)
}
if instancegroup == nil {
if warnUnmatched {
klog.Warningf("Found servergrp with no corresponding instance group %q", name)
}
continue
}
groups[instancegroup.ObjectMeta.Name], err = osBuildCloudInstanceGroup(c, cluster, instancegroup, grp, nodeMap)
for _, ig := range instancegroups {
var err error
groups[ig.ObjectMeta.Name], err = osBuildCloudInstanceGroup(c, cluster, ig, nodeMap)
if err != nil {
return nil, fmt.Errorf("error getting cloud instance group %q: %v", instancegroup.ObjectMeta.Name, err)
return nil, fmt.Errorf("error getting cloud instance group %q: %v", ig.ObjectMeta.Name, err)
}
}
return groups, nil
Expand Down
75 changes: 24 additions & 51 deletions upup/pkg/fi/cloudup/openstack/server_group.go
Expand Up @@ -84,80 +84,53 @@ func listServerGroups(c OpenstackCloud, opts servergroups.ListOptsBuilder) ([]se
}
}

// matchInstanceGroup filters a list of instancegroups for recognized cloud groups
func matchInstanceGroup(name string, clusterName string, instancegroups []*kops.InstanceGroup) (*kops.InstanceGroup, error) {
var instancegroup *kops.InstanceGroup
for _, g := range instancegroups {
var groupName string

switch g.Spec.Role {
case kops.InstanceGroupRoleControlPlane, kops.InstanceGroupRoleNode, kops.InstanceGroupRoleBastion:
groupName = clusterName + "-" + g.ObjectMeta.Name
default:
klog.Warningf("Ignoring InstanceGroup of unknown role %q", g.Spec.Role)
continue
}

if name == groupName {
if instancegroup != nil {
return nil, fmt.Errorf("found multiple instance groups matching servergrp %q", groupName)
}
instancegroup = g
}
}

return instancegroup, nil
}

func osBuildCloudInstanceGroup(c OpenstackCloud, cluster *kops.Cluster, ig *kops.InstanceGroup, g servergroups.ServerGroup, nodeMap map[string]*v1.Node) (*cloudinstances.CloudInstanceGroup, error) {
newLaunchConfigName := g.Name
func osBuildCloudInstanceGroup(c OpenstackCloud, cluster *kops.Cluster, ig *kops.InstanceGroup, nodeMap map[string]*v1.Node) (*cloudinstances.CloudInstanceGroup, error) {
cg := &cloudinstances.CloudInstanceGroup{
HumanName: newLaunchConfigName,
HumanName: ig.Name,
InstanceGroup: ig,
MinSize: int(fi.ValueOf(ig.Spec.MinSize)),
TargetSize: int(fi.ValueOf(ig.Spec.MinSize)), // TODO: Retrieve the target size from OpenStack?
TargetSize: int(fi.ValueOf(ig.Spec.MinSize)),
MaxSize: int(fi.ValueOf(ig.Spec.MaxSize)),
Raw: &g,
Raw: cluster,
}
for _, i := range g.Members {
instanceId := i
if instanceId == "" {
klog.Warningf("ignoring instance with no instance id: %s", i)

instances, err := c.ListInstances(servers.ListOpts{
Name: fmt.Sprintf("^%s", ig.Name),
})
if err != nil {
return nil, err
}
for _, instance := range instances {
value, ok := instance.Metadata[TagKopsInstanceGroup]
if !ok || value != ig.Name {
continue
}
server, err := servers.Get(c.ComputeClient(), instanceId).Extract()
if err != nil {
return nil, fmt.Errorf("Failed to get instance group member: %v", err)
}
igObservedGeneration := server.Metadata[INSTANCE_GROUP_GENERATION]
clusterObservedGeneration := server.Metadata[CLUSTER_GENERATION]
igObservedGeneration := instance.Metadata[INSTANCE_GROUP_GENERATION]
clusterObservedGeneration := instance.Metadata[CLUSTER_GENERATION]
observedName := fmt.Sprintf("%s-%s", clusterObservedGeneration, igObservedGeneration)
generationName := fmt.Sprintf("%d-%d", cluster.GetGeneration(), ig.Generation)

status := cloudinstances.CloudInstanceStatusUpToDate
if generationName != observedName || server.Status == errorStatus {
if generationName != observedName || instance.Status == errorStatus {
status = cloudinstances.CloudInstanceStatusNeedsUpdate
}
cm, err := cg.NewCloudInstance(instanceId, status, nodeMap[instanceId])
cm, err := cg.NewCloudInstance(instance.ID, status, nodeMap[instance.ID])
if err != nil {
return nil, fmt.Errorf("error creating cloud instance group member: %v", err)
}

if server.Flavor["original_name"] != nil {
cm.MachineType = server.Flavor["original_name"].(string)
if instance.Flavor["original_name"] != nil {
cm.MachineType = instance.Flavor["original_name"].(string)
}

ip, err := GetServerFixedIP(server, server.Metadata[TagKopsNetwork])
ip, err := GetServerFixedIP(&instance, instance.Metadata[TagKopsNetwork])
if err != nil {
klog.Warningf("Unable to find fixed ip for %s: %v", server.Name, err)
klog.Warningf("Unable to find fixed ip for %s: %v", instance.Name, err)
}

cm.PrivateIP = ip

cm.Roles = []string{server.Metadata["KopsRole"]}

cm.State = cloudinstances.State(server.Status)

cm.Roles = []string{instance.Metadata["KopsRole"]}
cm.State = cloudinstances.State(instance.Status)
}
return cg, nil
}
Expand Down

0 comments on commit 529670c

Please sign in to comment.