diff --git a/data/data/libvirt/bootstrap/main.tf b/data/data/libvirt/bootstrap/main.tf index a28d7c744ba..e6cb6966f74 100644 --- a/data/data/libvirt/bootstrap/main.tf +++ b/data/data/libvirt/bootstrap/main.tf @@ -1,15 +1,15 @@ resource "libvirt_volume" "bootstrap" { - name = "bootstrap" + name = "${var.cluster_name}-bootstrap" base_volume_id = "${var.base_volume_id}" } resource "libvirt_ignition" "bootstrap" { - name = "bootstrap.ign" + name = "${var.cluster_name}-bootstrap.ign" content = "${var.ignition}" } resource "libvirt_domain" "bootstrap" { - name = "bootstrap" + name = "${var.cluster_name}-bootstrap" memory = "2048" diff --git a/data/data/libvirt/main.tf b/data/data/libvirt/main.tf index a66156a6f3f..1be4670be9d 100644 --- a/data/data/libvirt/main.tf +++ b/data/data/libvirt/main.tf @@ -2,17 +2,18 @@ provider "libvirt" { uri = "${var.tectonic_libvirt_uri}" } -module "libvirt_base_volume" { +module "volume" { source = "./volume" - image = "${var.tectonic_os_image}" + cluster_name = "${var.tectonic_cluster_name}" + image = "${var.tectonic_os_image}" } module "bootstrap" { source = "./bootstrap" addresses = ["${var.tectonic_libvirt_bootstrap_ip}"] - base_volume_id = "${module.libvirt_base_volume.coreos_base_volume_id}" + base_volume_id = "${module.volume.coreos_base_volume_id}" cluster_name = "${var.tectonic_cluster_name}" ignition = "${var.ignition_bootstrap}" network_id = "${libvirt_network.tectonic_net.id}" @@ -20,22 +21,22 @@ module "bootstrap" { resource "libvirt_volume" "master" { count = "${var.tectonic_master_count}" - name = "master${count.index}" - base_volume_id = "${module.libvirt_base_volume.coreos_base_volume_id}" + name = "${var.tectonic_cluster_name}-master-${count.index}" + base_volume_id = "${module.volume.coreos_base_volume_id}" } resource "libvirt_ignition" "master" { - name = "master.ign" + name = "${var.tectonic_cluster_name}-master.ign" content = "${var.ignition_master}" } resource "libvirt_ignition" "worker" { - name = "worker.ign" + name = "${var.tectonic_cluster_name}-worker.ign" content = "${var.ignition_worker}" } resource "libvirt_network" "tectonic_net" { - name = "${var.tectonic_libvirt_network_name}" + name = "${var.tectonic_cluster_name}" mode = "nat" bridge = "${var.tectonic_libvirt_network_if}" @@ -67,7 +68,7 @@ resource "libvirt_network" "tectonic_net" { resource "libvirt_domain" "master" { count = "${var.tectonic_master_count}" - name = "master${count.index}" + name = "${var.tectonic_cluster_name}-master-${count.index}" memory = "3072" vcpu = "2" diff --git a/data/data/libvirt/variables-libvirt.tf b/data/data/libvirt/variables-libvirt.tf index e40ff850378..ce4454addce 100644 --- a/data/data/libvirt/variables-libvirt.tf +++ b/data/data/libvirt/variables-libvirt.tf @@ -8,11 +8,6 @@ variable "tectonic_libvirt_uri" { description = "libvirt connection URI" } -variable "tectonic_libvirt_network_name" { - type = "string" - description = "Name of the libvirt network to create" -} - variable "tectonic_libvirt_network_if" { type = "string" description = "The name of the bridge to use" diff --git a/data/data/libvirt/volume/main.tf b/data/data/libvirt/volume/main.tf index 73ea8166dc9..6bdae765744 100644 --- a/data/data/libvirt/volume/main.tf +++ b/data/data/libvirt/volume/main.tf @@ -1,4 +1,4 @@ resource "libvirt_volume" "coreos_base" { - name = "coreos_base" + name = "${var.cluster_name}-base" source = "${var.image}" } diff --git a/data/data/libvirt/volume/variables.tf b/data/data/libvirt/volume/variables.tf index 43471f87961..151bba7c88d 100644 --- a/data/data/libvirt/volume/variables.tf +++ b/data/data/libvirt/volume/variables.tf @@ -1,3 +1,8 @@ +variable "cluster_name" { + type = "string" + description = "The name of the cluster." +} + variable "image" { description = "The URL of the OS disk image" type = "string" diff --git a/docs/user/environment-variables.md b/docs/user/environment-variables.md index a2d317248e5..ab774bebcbd 100644 --- a/docs/user/environment-variables.md +++ b/docs/user/environment-variables.md @@ -17,6 +17,9 @@ The installer accepts a number of environment variable that allow the interactiv * `OPENSHIFT_INSTALL_CLUSTER_NAME`: The name of the cluster. This will be used when generating sub-domains. + + For libvirt, choose a name that is unique enough to be used as a prefix during cluster deletion. + For example, if you use `demo` as your cluster name, `openshift-install destroy cluster` may destroy all domains, networks, pools, and volumes that begin with `demo`. * `OPENSHIFT_INSTALL_EMAIL_ADDRESS`: The email address of the cluster administrator. This will be used to log in to the console. diff --git a/pkg/asset/installconfig/clustername.go b/pkg/asset/installconfig/clustername.go index 4ff7c6380ee..ab2788dba68 100644 --- a/pkg/asset/installconfig/clustername.go +++ b/pkg/asset/installconfig/clustername.go @@ -25,7 +25,7 @@ func (a *clusterName) Generate(asset.Parents) error { &survey.Question{ Prompt: &survey.Input{ Message: "Cluster Name", - Help: "The name of the cluster. This will be used when generating sub-domains.", + Help: "The name of the cluster. This will be used when generating sub-domains.\n\nFor libvirt, choose a name that is unique enough to be used as a prefix during cluster deletion. For example, if you use 'demo' as your cluster name, `openshift-install destroy cluster` may destroy all domains, networks, pools, and volumes that begin with 'demo'.", }, Validate: survey.ComposeValidators(survey.Required, func(ans interface{}) error { return validate.DomainName(ans.(string)) diff --git a/pkg/asset/installconfig/installconfig.go b/pkg/asset/installconfig/installconfig.go index b8d91e5ac45..34750d4cee9 100644 --- a/pkg/asset/installconfig/installconfig.go +++ b/pkg/asset/installconfig/installconfig.go @@ -104,7 +104,6 @@ func (a *InstallConfig) Generate(parents asset.Parents) error { a.Config.OpenStack = platform.OpenStack case platform.Libvirt != nil: a.Config.Libvirt = platform.Libvirt - a.Config.Libvirt.Network.Name = clusterName.ClusterName numberOfMasters = 1 numberOfWorkers = 1 default: diff --git a/pkg/asset/machines/libvirt/machines.go b/pkg/asset/machines/libvirt/machines.go index e1219358c3b..7b81740c233 100644 --- a/pkg/asset/machines/libvirt/machines.go +++ b/pkg/asset/machines/libvirt/machines.go @@ -31,7 +31,7 @@ func Machines(config *types.InstallConfig, pool *types.MachinePool, role, userDa if pool.Replicas != nil { total = *pool.Replicas } - provider := provider(platform, pool.Name) + provider := provider(clustername, platform, pool.Name) var machines []clusterapi.Machine for idx := int64(0); idx < total; idx++ { machine := clusterapi.Machine{ @@ -41,7 +41,7 @@ func Machines(config *types.InstallConfig, pool *types.MachinePool, role, userDa }, ObjectMeta: metav1.ObjectMeta{ Namespace: "openshift-cluster-api", - Name: fmt.Sprintf("%s%d", pool.Name, idx), + Name: fmt.Sprintf("%s-%s-%d", clustername, pool.Name, idx), Labels: map[string]string{ "sigs.k8s.io/cluster-api-cluster": clustername, "sigs.k8s.io/cluster-api-machine-role": role, @@ -61,7 +61,7 @@ func Machines(config *types.InstallConfig, pool *types.MachinePool, role, userDa return machines, nil } -func provider(platform *libvirt.Platform, name string) *libvirtprovider.LibvirtMachineProviderConfig { +func provider(clusterName string, platform *libvirt.Platform, name string) *libvirtprovider.LibvirtMachineProviderConfig { return &libvirtprovider.LibvirtMachineProviderConfig{ TypeMeta: metav1.TypeMeta{ APIVersion: "libvirtproviderconfig.k8s.io/v1alpha1", @@ -69,12 +69,12 @@ func provider(platform *libvirt.Platform, name string) *libvirtprovider.LibvirtM }, DomainMemory: 2048, DomainVcpu: 2, - IgnKey: fmt.Sprintf("/var/lib/libvirt/images/%s.ign", name), + IgnKey: fmt.Sprintf("/var/lib/libvirt/images/%s-%s.ign", clusterName, name), Volume: &libvirtprovider.Volume{ PoolName: "default", - BaseVolumeID: "/var/lib/libvirt/images/coreos_base", + BaseVolumeID: fmt.Sprintf("/var/lib/libvirt/images/%s-base", clusterName), }, - NetworkInterfaceName: platform.Network.Name, + NetworkInterfaceName: clusterName, NetworkInterfaceAddress: platform.Network.IPRange, Autostart: false, URI: platform.URI, diff --git a/pkg/asset/machines/libvirt/machinesets.go b/pkg/asset/machines/libvirt/machinesets.go index 2d8dace7038..713ba0f8237 100644 --- a/pkg/asset/machines/libvirt/machinesets.go +++ b/pkg/asset/machines/libvirt/machinesets.go @@ -32,7 +32,7 @@ func MachineSets(config *types.InstallConfig, pool *types.MachinePool, role, use total = *pool.Replicas } - provider := provider(platform, pool.Name) + provider := provider(clustername, platform, pool.Name) name := fmt.Sprintf("%s-%s-%d", clustername, pool.Name, 0) mset := clusterapi.MachineSet{ TypeMeta: metav1.TypeMeta{ diff --git a/pkg/destroy/libvirt/libvirt_prefix_deprovision.go b/pkg/destroy/libvirt/libvirt.go similarity index 51% rename from pkg/destroy/libvirt/libvirt_prefix_deprovision.go rename to pkg/destroy/libvirt/libvirt.go index 3ebf6b1e692..37dfff0094a 100644 --- a/pkg/destroy/libvirt/libvirt_prefix_deprovision.go +++ b/pkg/destroy/libvirt/libvirt.go @@ -3,14 +3,11 @@ package libvirt import ( - "os" "strings" - "time" libvirt "github.com/libvirt/libvirt-go" "github.com/pkg/errors" "github.com/sirupsen/logrus" - "k8s.io/apimachinery/pkg/util/wait" "github.com/openshift/installer/pkg/destroy" "github.com/openshift/installer/pkg/types" @@ -36,17 +33,12 @@ var ClusterNamePrefixFilter = func(clustername string) filterFunc { // names except `default`. var AlwaysTrueFilter = func() filterFunc { return func(name string) bool { - if name == "default" { - return false - } - return true + return name != "default" } } -// deleteFunc type is the interface a function needs to implement to be called as a goroutine. -// The (bool, error) return type mimics wait.ExponentialBackoff where the bool indicates successful -// completion, and the error is for unrecoverable errors. -type deleteFunc func(conn *libvirt.Connect, filter filterFunc, logger logrus.FieldLogger) (bool, error) +// deleteFunc is the interface a function needs to implement to be delete resources. +type deleteFunc func(conn *libvirt.Connect, filter filterFunc, logger logrus.FieldLogger) error // ClusterUninstaller holds the various options for the cluster we want to delete. type ClusterUninstaller struct { @@ -55,104 +47,82 @@ type ClusterUninstaller struct { Logger logrus.FieldLogger } -// Run is the entrypoint to start the uninstall process +// Run is the entrypoint to start the uninstall process. func (o *ClusterUninstaller) Run() error { - deleteFuncs := map[string]deleteFunc{} - populateDeleteFuncs(deleteFuncs) - returnChannel := make(chan string) - conn, err := libvirt.NewConnect(o.LibvirtURI) if err != nil { return errors.Wrap(err, "failed to connect to Libvirt daemon") } - // launch goroutines - for name, function := range deleteFuncs { - go deleteRunner(name, function, conn, o.Filter, o.Logger, returnChannel) - } - - // wait for them to finish - for i := 0; i < len(deleteFuncs); i++ { - select { - case res := <-returnChannel: - o.Logger.Debugf("goroutine %v complete", res) + for _, del := range []deleteFunc{ + deleteDomains, + deleteNetwork, + deleteVolumes, + } { + err = del(conn, o.Filter, o.Logger) + if err != nil { + return err } } return nil } -func deleteRunner(deleteFuncName string, dFunction deleteFunc, conn *libvirt.Connect, filter filterFunc, logger logrus.FieldLogger, channel chan string) { - backoffSettings := wait.Backoff{ - Duration: time.Second * 10, - Factor: 1.3, - Steps: 100, - } - - err := wait.ExponentialBackoff(backoffSettings, func() (bool, error) { - return dFunction(conn, filter, logger) - }) - - if err != nil { - logger.Fatalf("Unrecoverable error/timed out: %v", err) - os.Exit(1) +// deleteDomains calls deleteDomainsSinglePass until it finds no +// matching domains. This guards against the machine-API launching +// additional nodes after the initial list call. We continue deleting +// domains until we either hit an error or we have a list call with no +// matching domains. +func deleteDomains(conn *libvirt.Connect, filter filterFunc, logger logrus.FieldLogger) error { + logger.Debug("Deleting libvirt domains") + var err error + nothingToDelete := false + for !nothingToDelete { + nothingToDelete, err = deleteDomainsSinglePass(conn, filter, logger) + if err != nil { + return err + } } - - // record that the goroutine has run to completion - channel <- deleteFuncName - return -} - -// populateDeleteFuncs is the list of functions that will be launched as goroutines -func populateDeleteFuncs(funcs map[string]deleteFunc) { - funcs["deleteDomains"] = deleteDomains - funcs["deleteVolumes"] = deleteVolumes - funcs["deleteNetwork"] = deleteNetwork + return nil } -func deleteDomains(conn *libvirt.Connect, filter filterFunc, logger logrus.FieldLogger) (bool, error) { - logger.Debug("Deleting libvirt domains") - defer logger.Debugf("Exiting deleting libvirt domains") - +func deleteDomainsSinglePass(conn *libvirt.Connect, filter filterFunc, logger logrus.FieldLogger) (nothingToDelete bool, err error) { domains, err := conn.ListAllDomains(0) if err != nil { - logger.Errorf("Error listing domains: %v", err) - return false, nil + return false, errors.Wrap(err, "list domains") } + nothingToDelete = true for _, domain := range domains { defer domain.Free() dName, err := domain.GetName() if err != nil { - logger.Errorf("Error getting name for domain: %v", err) - return false, nil + return false, errors.Wrap(err, "get domain name") } if !filter(dName) { continue } + nothingToDelete = false if err := domain.Destroy(); err != nil { - logger.Errorf("Error destroying domain %s: %v", dName, err) - return false, nil + return false, errors.Wrapf(err, "destroy domain %q", dName) } if err := domain.Undefine(); err != nil { - logger.Errorf("Error un-defining domain %s: %v", dName, err) - return false, nil + return false, errors.Wrapf(err, "undefine domain %q", dName) } logger.WithField("domain", dName).Info("Deleted domain") } - return true, nil + + return nothingToDelete, nil } -func deleteVolumes(conn *libvirt.Connect, filter filterFunc, logger logrus.FieldLogger) (bool, error) { +func deleteVolumes(conn *libvirt.Connect, filter filterFunc, logger logrus.FieldLogger) error { logger.Debug("Deleting libvirt volumes") - defer logger.Debugf("Exiting deleting libvirt volumes") pools, err := conn.ListStoragePools() if err != nil { - logger.Errorf("Error listing storage pools: %v", err) - return false, nil + return errors.Wrap(err, "list storage pools") } tpool := "default" @@ -164,8 +134,7 @@ func deleteVolumes(conn *libvirt.Connect, filter filterFunc, logger logrus.Field } pool, err := conn.LookupStoragePoolByName(tpool) if err != nil { - logger.Errorf("Error getting storage pool %s: %v", tpool, err) - return false, nil + return errors.Wrapf(err, "get storage pool %q", tpool) } defer pool.Free() @@ -174,51 +143,44 @@ func deleteVolumes(conn *libvirt.Connect, filter filterFunc, logger logrus.Field // delete all vols that return true from filter. vols, err := pool.ListAllStorageVolumes(0) if err != nil { - logger.Errorf("Error listing storage volumes in %s: %v", tpool, err) - return false, nil + return errors.Wrapf(err, "list volumes in %q", tpool) } for _, vol := range vols { defer vol.Free() vName, err := vol.GetName() if err != nil { - logger.Errorf("Error getting name for volume: %v", err) - return false, nil + return errors.Wrapf(err, "get volume names in %q", tpool) } if !filter(vName) { continue } if err := vol.Delete(0); err != nil { - logger.Errorf("Error deleting volume %s: %v", vName, err) - return false, nil + return errors.Wrapf(err, "delete volume %q from %q", vName, tpool) } logger.WithField("volume", vName).Info("Deleted volume") } default: // blow away entire pool. if err := pool.Destroy(); err != nil { - logger.Errorf("Error destroying pool %s: %v", tpool, err) - return false, nil + return errors.Wrapf(err, "destroy pool %q", tpool) } if err := pool.Undefine(); err != nil { - logger.Errorf("Error undefining pool %s: %v", tpool, err) - return false, nil + return errors.Wrapf(err, "undefine pool %q", tpool) } logger.WithField("pool", tpool).Info("Deleted pool") } - return true, nil + return nil } -func deleteNetwork(conn *libvirt.Connect, filter filterFunc, logger logrus.FieldLogger) (bool, error) { +func deleteNetwork(conn *libvirt.Connect, filter filterFunc, logger logrus.FieldLogger) error { logger.Debug("Deleting libvirt network") - defer logger.Debugf("Exiting deleting libvirt network") networks, err := conn.ListNetworks() if err != nil { - logger.Errorf("Error listing networks: %v", err) - return false, nil + return errors.Wrap(err, "list networks") } for _, nName := range networks { @@ -227,30 +189,27 @@ func deleteNetwork(conn *libvirt.Connect, filter filterFunc, logger logrus.Field } network, err := conn.LookupNetworkByName(nName) if err != nil { - logger.Errorf("Error getting network %s: %v", nName, err) - return false, nil + return errors.Wrapf(err, "get network %q", nName) } defer network.Free() if err := network.Destroy(); err != nil { - logger.Errorf("Error destroying network %s: %v", nName, err) - return false, nil + return errors.Wrapf(err, "destroy network %q", nName) } if err := network.Undefine(); err != nil { - logger.Errorf("Error undefining network %s: %v", nName, err) - return false, nil + return errors.Wrapf(err, "undefine network %q", nName) } logger.WithField("network", nName).Info("Deleted network") } - return true, nil + return nil } // New returns libvirt Uninstaller from ClusterMetadata. func New(logger logrus.FieldLogger, metadata *types.ClusterMetadata) (destroy.Destroyer, error) { return &ClusterUninstaller{ LibvirtURI: metadata.ClusterPlatformMetadata.Libvirt.URI, - Filter: AlwaysTrueFilter(), //TODO: change to ClusterNamePrefixFilter when all resources are prefixed. + Filter: ClusterNamePrefixFilter(metadata.ClusterName), Logger: logger, }, nil } diff --git a/pkg/tfvars/libvirt/libvirt.go b/pkg/tfvars/libvirt/libvirt.go index 3d0f9c80061..08df47a2ed7 100644 --- a/pkg/tfvars/libvirt/libvirt.go +++ b/pkg/tfvars/libvirt/libvirt.go @@ -24,7 +24,6 @@ type Libvirt struct { // Network describes a libvirt network configuration. type Network struct { - Name string `json:"tectonic_libvirt_network_name,omitempty" yaml:"name"` IfName string `json:"tectonic_libvirt_network_if,omitempty" yaml:"ifName"` IPRange string `json:"tectonic_libvirt_ip_range,omitempty" yaml:"ipRange"` } diff --git a/pkg/tfvars/tfvars.go b/pkg/tfvars/tfvars.go index ad6431af83e..1fa70bb2f2a 100644 --- a/pkg/tfvars/tfvars.go +++ b/pkg/tfvars/tfvars.go @@ -109,7 +109,6 @@ func TFVars(cfg *types.InstallConfig, bootstrapIgn, masterIgn, workerIgn string) config.Libvirt = libvirt.Libvirt{ URI: cfg.Platform.Libvirt.URI, Network: libvirt.Network{ - Name: cfg.Platform.Libvirt.Network.Name, IfName: cfg.Platform.Libvirt.Network.IfName, IPRange: cfg.Platform.Libvirt.Network.IPRange, }, diff --git a/pkg/types/libvirt/network.go b/pkg/types/libvirt/network.go index 217c0aa4bdd..98198f7ff20 100644 --- a/pkg/types/libvirt/network.go +++ b/pkg/types/libvirt/network.go @@ -2,8 +2,6 @@ package libvirt // Network is the configuration of the libvirt network. type Network struct { - // Name is the name of the nework. - Name string `json:"name"` // IfName is the name of the network interface. IfName string `json:"if"` // IPRange is the range of IPs to use.