Skip to content
Merged
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
2 changes: 1 addition & 1 deletion data/data/openstack/topology/private-network.tf
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ resource "openstack_networking_network_v2" "openshift-private" {
count = var.machines_subnet_id == "" ? 1 : 0
name = "${var.cluster_id}-openshift"
admin_state_up = "true"
tags = ["openshiftClusterID=${var.cluster_id}"]
tags = ["openshiftClusterID=${var.cluster_id}", "${var.cluster_id}-primaryClusterNetwork"]
}

resource "openstack_networking_subnet_v2" "nodes" {
Expand Down
84 changes: 80 additions & 4 deletions pkg/destroy/openstack/openstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/gophercloud/gophercloud/openstack/imageservice/v2/images"
"github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/apiversions"
"github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/loadbalancers"
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/attributestags"
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/floatingips"
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/routers"
sg "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/groups"
Expand All @@ -25,6 +26,7 @@ import (
"github.com/gophercloud/gophercloud/openstack/objectstorage/v1/containers"
"github.com/gophercloud/gophercloud/openstack/objectstorage/v1/objects"
"github.com/gophercloud/utils/openstack/clientconfig"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"k8s.io/apimachinery/pkg/util/wait"
)
Expand Down Expand Up @@ -59,15 +61,18 @@ type ClusterUninstaller struct {
Cloud string
// Filter contains the openshiftClusterID to filter tags
Filter Filter
Logger logrus.FieldLogger
// InfraID contains unique cluster identifier
InfraID string
Logger logrus.FieldLogger
}

// New returns an OpenStack destroyer from ClusterMetadata.
func New(logger logrus.FieldLogger, metadata *types.ClusterMetadata) (providers.Destroyer, error) {
return &ClusterUninstaller{
Cloud: metadata.ClusterPlatformMetadata.OpenStack.Cloud,
Filter: metadata.ClusterPlatformMetadata.OpenStack.Identifier,
Logger: logger,
Cloud: metadata.ClusterPlatformMetadata.OpenStack.Cloud,
Filter: metadata.ClusterPlatformMetadata.OpenStack.Identifier,
InfraID: metadata.InfraID,
Logger: logger,
}, nil
}

Expand Down Expand Up @@ -110,6 +115,12 @@ func (o *ClusterUninstaller) Run() error {
}
}

// we need to untag the custom network if it was provided by the user
err := untagRunner(opts, o.InfraID, o.Logger)
if err != nil {
return err
}

return nil
}

Expand Down Expand Up @@ -998,3 +1009,68 @@ func deleteImages(opts *clientconfig.ClientOpts, filter Filter, logger logrus.Fi
}
return true, nil
}

func untagRunner(opts *clientconfig.ClientOpts, infraID string, logger logrus.FieldLogger) error {
backoffSettings := wait.Backoff{
Duration: time.Second * 10,
Steps: 25,
}

err := wait.ExponentialBackoff(backoffSettings, func() (bool, error) {
return untagPrimaryNetwork(opts, infraID, logger)
})
if err != nil {
if err == wait.ErrWaitTimeout {
return err
}
return errors.Errorf("Unrecoverable error: %v", err)
}

return nil
}

// untagNetwork removes the tag from the primary cluster network based on unfra id
func untagPrimaryNetwork(opts *clientconfig.ClientOpts, infraID string, logger logrus.FieldLogger) (bool, error) {
networkTag := infraID + "-primaryClusterNetwork"

logger.Debugf("Removing tag %v from openstack networks", networkTag)
defer logger.Debug("Exiting untagging openstack networks")

conn, err := clientconfig.NewServiceClient("network", opts)
if err != nil {
logger.Debug(err)
return false, nil
}

listOpts := networks.ListOpts{
Tags: networkTag,
}

allPages, err := networks.List(conn, listOpts).AllPages()
if err != nil {
logger.Debug(err)
return false, nil
}

allNetworks, err := networks.ExtractNetworks(allPages)
if err != nil {
logger.Debug(err)
return false, nil
}

if len(allNetworks) > 1 {
return false, errors.Errorf("More than one network with tag %v", networkTag)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why make it an error? That will cause the untagRunner to enter a loop to perform an action it can't do.
IMO, as opposed to deleting resources when we're not sure we were the ones creating them, it's OK to remove (internally used) tags from resources.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If we return an error, then untagRunner stops working immediately, no matter how many attempts there are left. So, for the runner it basically means - stop looping and return the error.
https://github.com/kubernetes/apimachinery/blob/release-1.17/pkg/util/wait/wait.go#L293

Despite the fact we consider the tag to be unique because it contains an infra ID, users can still add the tag to other networks. So removing them from all networks can lead to unpleasant side effects, which means we must stop the execution and do not try to repeat the untagging.

}

if len(allNetworks) == 0 {
// The network has already been deleted.
return true, nil
}

err = attributestags.Delete(conn, "networks", allNetworks[0].ID, networkTag).ExtractErr()
if err != nil {
return false, nil
}

return true, nil
}
29 changes: 29 additions & 0 deletions pkg/tfvars/openstack/openstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/gophercloud/gophercloud/openstack"
"github.com/gophercloud/gophercloud/openstack/identity/v3/tokens"
"github.com/gophercloud/gophercloud/openstack/imageservice/v2/images"
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/attributestags"
"github.com/gophercloud/gophercloud/openstack/networking/v2/subnets"
"github.com/gophercloud/utils/openstack/clientconfig"
"github.com/openshift/installer/pkg/rhcos"
Expand Down Expand Up @@ -152,6 +153,15 @@ func TFVars(masterConfig *v1alpha1.OpenstackProviderSpec, cloud string, external
if err != nil {
return nil, err
}

// Make sure that the network has the primary cluster network tag.
// In the case of multiple networks this tag is required for
// cluster-api-provider-openstack to define which ip address to set as
// the primary one.
err = setNetworkTag(cloud, cfg.MachinesNetwork, infraID+"-primaryClusterNetwork")
if err != nil {
return nil, err
}
}

return json.MarshalIndent(cfg, "", " ")
Expand Down Expand Up @@ -265,3 +275,22 @@ func getNetworkFromSubnet(cloud string, subnetID string) (string, error) {

return subnet.NetworkID, nil
}

// setNetworkTag sets a tag for the network
func setNetworkTag(cloud string, networkID string, networkTag string) error {
opts := &clientconfig.ClientOpts{
Cloud: cloud,
}

networkClient, err := clientconfig.NewServiceClient("network", opts)
if err != nil {
return err
}

err = attributestags.Add(networkClient, "networks", networkID, networkTag).ExtractErr()
if err != nil {
return err
}

return nil
}