cmd/destroy: delete cluster asset after destroying cluster

cmd/openshift-install/create.go

@@ -34,41 +34,58 @@ type target struct {
 	assets  []asset.WritableAsset
 }
 
-var targets = []target{{
-	name: "Install Config",
-	command: &cobra.Command{
-		Use:   "install-config",
-		Short: "Generates the Install Config asset",
-		Long:  "",
-	},
-	assets: []asset.WritableAsset{&installconfig.InstallConfig{}},
-}, {
-	name: "Manifests",
-	command: &cobra.Command{
-		Use:   "manifests",
-		Short: "Generates the Kubernetes manifests",
-		Long:  "",
-	},
-	assets: []asset.WritableAsset{&manifests.Manifests{}, &manifests.Tectonic{}},
-}, {
-	name: "Ignition Configs",
-	command: &cobra.Command{
-		Use:   "ignition-configs",
-		Short: "Generates the Ignition Config asset",
-		Long:  "",
-	},
-	assets: []asset.WritableAsset{&bootstrap.Bootstrap{}, &machine.Master{}, &machine.Worker{}},
-}, {
-	name: "Cluster",
-	command: &cobra.Command{
-		Use:   "cluster",
-		Short: "Create an OpenShift cluster",
-		PostRunE: func(_ *cobra.Command, _ []string) error {
-			return destroyBootstrap(context.Background(), rootOpts.dir)
+// each target is a variable to preserve the order when creating subcommands and still
+// allow other functions to directly access each target individually.
+var (
+	installConfigTarget = target{
+		name: "Install Config",
+		command: &cobra.Command{
+			Use:   "install-config",
+			Short: "Generates the Install Config asset",
+			// FIXME: add longer descriptions for our commands with examples for better UX.
+			// Long:  "",
 		},
-	},
-	assets: []asset.WritableAsset{&cluster.TerraformVariables{}, &kubeconfig.Admin{}, &cluster.Cluster{}},
-}}
+		assets: []asset.WritableAsset{&installconfig.InstallConfig{}},
+	}
+
+	manifestsTarget = target{
+		name: "Manifests",
+		command: &cobra.Command{
+			Use:   "manifests",
+			Short: "Generates the Kubernetes manifests",
+			// FIXME: add longer descriptions for our commands with examples for better UX.
+			// Long:  "",
+		},
+		assets: []asset.WritableAsset{&manifests.Manifests{}, &manifests.Tectonic{}},
+	}
+
+	ignitionConfigsTarget = target{
+		name: "Ignition Configs",
+		command: &cobra.Command{
+			Use:   "ignition-configs",
+			Short: "Generates the Ignition Config asset",
+			// FIXME: add longer descriptions for our commands with examples for better UX.
+			// Long:  "",
+		},
+		assets: []asset.WritableAsset{&bootstrap.Bootstrap{}, &machine.Master{}, &machine.Worker{}},
+	}
+
+	clusterTarget = target{
+		name: "Cluster",
+		command: &cobra.Command{
+			Use:   "cluster",
+			Short: "Create an OpenShift cluster",
+			// FIXME: add longer descriptions for our commands with examples for better UX.
+			// Long:  "",
+			PostRunE: func(_ *cobra.Command, _ []string) error {
+				return destroyBootstrap(context.Background(), rootOpts.dir)
+			},
+		},
+		assets: []asset.WritableAsset{&cluster.TerraformVariables{}, &kubeconfig.Admin{}, &cluster.Cluster{}},
+	}
+
+	targets = []target{installConfigTarget, manifestsTarget, ignitionConfigsTarget, clusterTarget}
+)
 
 // Deprecated: Use 'create' subcommands instead.
 func newTargetsCmd() []*cobra.Command {
@@ -128,15 +145,6 @@ func runTargetCmd(targets ...asset.WritableAsset) func(cmd *cobra.Command, args
 				return err
 			}
 		}
-
-		if err := assetStore.Save(rootOpts.dir); err != nil {
-			return errors.Wrapf(err, "failed to write to state file")
-		}
-
-		if err := assetStore.Purge(targets); err != nil {
-			return errors.Wrapf(err, "failed to delete existing on-disk files")
-		}
-
 		return nil
 	}
 }

cmd/openshift-install/destroy.go

@@ -5,6 +5,7 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 
+	"github.com/openshift/installer/pkg/asset"
 	"github.com/openshift/installer/pkg/destroy"
 	"github.com/openshift/installer/pkg/destroy/bootstrap"
 	_ "github.com/openshift/installer/pkg/destroy/libvirt"
@@ -49,6 +50,16 @@ func runDestroyCmd(cmd *cobra.Command, args []string) error {
 		return errors.Wrap(err, "Failed to destroy cluster")
 
 	}
+
+	store, err := asset.NewStore(rootOpts.dir)
+	if err != nil {
+		return errors.Wrapf(err, "failed to create asset store")
+	}
+	for _, asset := range clusterTarget.assets {
+		if err := store.Destroy(asset); err != nil {
+			return errors.Wrapf(err, "failed to destroy asset %q", asset.Name())
+		}
+	}
 	return nil
 }
 

pkg/asset/asset.go

@@ -1,11 +1,13 @@
 package asset
 
 import (
+	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 // Asset used to install OpenShift.
@@ -55,3 +57,41 @@ func PersistToFile(asset WritableAsset, directory string) error {
 	}
 	return nil
 }
+
+// deleteAssetFromDisk removes all the files for asset from disk.
+// this is function is not safe for calling concurrently on the same directory.
+func deleteAssetFromDisk(asset WritableAsset, directory string) error {
+	logrus.Debugf("Purging asset %q from disk", asset.Name())
+	for _, f := range asset.Files() {
+		path := filepath.Join(directory, f.Filename)
+		if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
+			return errors.Wrap(err, "failed to remove file")
+		}
+
+		dir := filepath.Dir(path)
+		ok, err := isDirEmpty(dir)
+		if err != nil && !os.IsNotExist(err) {
+			return errors.Wrap(err, "failed to read directory")
+		}
+		if ok {
+			if err := os.Remove(dir); err != nil {
+				return errors.Wrap(err, "failed to remove directory")
+			}
+		}
+	}
+	return nil
+}
+
+func isDirEmpty(name string) (bool, error) {
+	f, err := os.Open(name)
+	if err != nil {
+		return false, err
+	}
+	defer f.Close()
+
+	_, err = f.Readdirnames(1) // Or f.Readdir(1)
+	if err == io.EOF {
+		return true, nil
+	}
+	return false, err // Either not empty or error, suits both cases
+}

pkg/asset/store.go

@@ -21,12 +21,9 @@ type Store interface {
 	// dependencies if necessary.
 	Fetch(Asset) error
 
-	// Save dumps the entire state map into a file
-	Save(dir string) error
-
-	// Purge deletes the on-disk assets that are consumed already.
-	// E.g., install-config.yml will be deleted after fetching 'manifests'.
-	Purge(excluded []WritableAsset) error
+	// Destroy removes the asset from all its internal state and also from
+	// disk if possible.
+	Destroy(Asset) error
 }
 
 // assetState includes an asset and a boolean that indicates
@@ -42,7 +39,8 @@ type StoreImpl struct {
 	assets          map[reflect.Type]assetState
 	stateFileAssets map[string]json.RawMessage
 	fileFetcher     *fileFetcher
-	onDiskAssets    []WritableAsset // This records the on-disk assets that are loaded already, which will be cleaned up in the end.
+
+	markedForPurge []WritableAsset // This records the on-disk assets that are loaded already, which will be cleaned up in the end.
 }
 
 // NewStore returns an asset store that implements the Store interface.
@@ -53,25 +51,57 @@ func NewStore(dir string) (Store, error) {
 		assets:      make(map[reflect.Type]assetState),
 	}
 
-	if err := store.load(dir); err != nil {
+	if err := store.load(); err != nil {
 		return nil, err
 	}
-
 	return store, nil
 }
 
 // Fetch retrieves the state of the given asset, generating it and its
 // dependencies if necessary.
 func (s *StoreImpl) Fetch(asset Asset) error {
-	_, err := s.fetch(asset, "")
-	return err
+	if _, err := s.fetch(asset, ""); err != nil {
+		return err
+	}
+	if err := s.save(); err != nil {
+		return errors.Wrapf(err, "failed to save state")
+	}
+	if wa, ok := asset.(WritableAsset); ok {
+		return errors.Wrapf(s.purge([]WritableAsset{wa}), "failed to purge asset")
+	}
+	return nil
+}
+
+// Destroy removes the asset from all its internal state and also from
+// disk if possible.
+func (s *StoreImpl) Destroy(asset Asset) error {
+	if sa, ok := s.assets[reflect.TypeOf(asset)]; ok {
+		reflect.ValueOf(asset).Elem().Set(reflect.ValueOf(sa.asset).Elem())
+	} else if s.isAssetInState(asset) {
+		if err := s.loadAssetFromState(asset); err != nil {
+			return err
+		}
+	} else {
+		// nothing to do
+		return nil
+	}
+
+	if wa, ok := asset.(WritableAsset); ok {
+		if err := deleteAssetFromDisk(wa, s.directory); err != nil {
+			return err
+		}
+	}
+
+	delete(s.assets, reflect.TypeOf(asset))
+	delete(s.stateFileAssets, reflect.TypeOf(asset).String())
+	return s.save()
 }
 
 // load retrieves the state from the state file present in the given directory
 // and returns the assets map
-func (s *StoreImpl) load(dir string) error {
-	path := filepath.Join(dir, stateFileName)
-	assets := make(map[string]json.RawMessage)
+func (s *StoreImpl) load() error {
+	path := filepath.Join(s.directory, stateFileName)
+	assets := map[string]json.RawMessage{}
 	data, err := ioutil.ReadFile(path)
 	if err != nil {
 		if os.IsNotExist(err) {
@@ -87,33 +117,39 @@ func (s *StoreImpl) load(dir string) error {
 	return nil
 }
 
-// LoadAssetFromState renders the asset object arguments from the state file contents.
-func (s *StoreImpl) LoadAssetFromState(asset Asset) error {
+// loadAssetFromState renders the asset object arguments from the state file contents.
+func (s *StoreImpl) loadAssetFromState(asset Asset) error {
 	bytes, ok := s.stateFileAssets[reflect.TypeOf(asset).String()]
 	if !ok {
 		return errors.Errorf("asset %q is not found in the state file", asset.Name())
 	}
 	return json.Unmarshal(bytes, asset)
 }
 
-// IsAssetInState tests whether the asset is in the state file.
-func (s *StoreImpl) IsAssetInState(asset Asset) bool {
+// isAssetInState tests whether the asset is in the state file.
+func (s *StoreImpl) isAssetInState(asset Asset) bool {
 	_, ok := s.stateFileAssets[reflect.TypeOf(asset).String()]
 	return ok
 }
 
-// Save dumps the entire state map into a file
-func (s *StoreImpl) Save(dir string) error {
-	assetMap := make(map[string]Asset)
+// save dumps the entire state map into a file
+func (s *StoreImpl) save() error {
+	if s.stateFileAssets == nil {
+		s.stateFileAssets = map[string]json.RawMessage{}
+	}
 	for k, v := range s.assets {
-		assetMap[k.String()] = v.asset
+		data, err := json.MarshalIndent(v.asset, "", "    ")
+		if err != nil {
+			return err
+		}
+		s.stateFileAssets[k.String()] = json.RawMessage(data)
 	}
-	data, err := json.MarshalIndent(&assetMap, "", "    ")
+	data, err := json.MarshalIndent(s.stateFileAssets, "", "    ")
 	if err != nil {
 		return err
 	}
 
-	path := filepath.Join(dir, stateFileName)
+	path := filepath.Join(s.directory, stateFileName)
 	if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
 		return err
 	}
@@ -159,7 +195,7 @@ func (s *StoreImpl) fetch(asset Asset, indent string) (bool, error) {
 	}
 
 	// Try to find the asset from the state file.
-	foundInStateFile := s.IsAssetInState(asset)
+	foundInStateFile := s.isAssetInState(asset)
 	if foundInStateFile {
 		logrus.Debugf("%sFound %q in state file", indent, asset.Name())
 	}
@@ -174,7 +210,7 @@ func (s *StoreImpl) fetch(asset Asset, indent string) (bool, error) {
 		}
 		if foundOnDisk {
 			logrus.Infof("Consuming %q from target directory", asset.Name())
-			s.onDiskAssets = append(s.onDiskAssets, as)
+			s.markedForPurge = append(s.markedForPurge, as)
 		}
 	}
 
@@ -191,7 +227,7 @@ func (s *StoreImpl) fetch(asset Asset, indent string) (bool, error) {
 		logrus.Debugf("%sLoading %q from both state file and target directory", indent, asset.Name())
 
 		stateAsset := reflect.New(reflect.TypeOf(asset).Elem()).Interface().(Asset)
-		if err := s.LoadAssetFromState(stateAsset); err != nil {
+		if err := s.loadAssetFromState(stateAsset); err != nil {
 			return false, errors.Wrapf(err, "failed to load asset %q from state file", asset.Name())
 		}
 
@@ -202,7 +238,7 @@ func (s *StoreImpl) fetch(asset Asset, indent string) (bool, error) {
 		}
 	} else if foundInStateFile {
 		logrus.Debugf("%sLoading %q from state file", indent, asset.Name())
-		if err := s.LoadAssetFromState(asset); err != nil {
+		if err := s.loadAssetFromState(asset); err != nil {
 			return false, errors.Wrapf(err, "failed to load asset %q from state file", asset.Name())
 		}
 	} else if foundOnDisk {
@@ -214,13 +250,12 @@ func (s *StoreImpl) fetch(asset Asset, indent string) (bool, error) {
 	return dirty, nil
 }
 
-// Purge deletes the on-disk assets that are consumed already.
+// purge deletes the on-disk assets that are consumed already.
 // E.g., install-config.yml will be deleted after fetching 'manifests'.
 // The target assets are excluded.
-func (s *StoreImpl) Purge(excluded []WritableAsset) error {
+func (s *StoreImpl) purge(excluded []WritableAsset) error {
 	var toPurge []WritableAsset
-
-	for _, asset := range s.onDiskAssets {
+	for _, asset := range s.markedForPurge {
 		var found bool
 		for _, as := range excluded {
 			if reflect.TypeOf(as) == reflect.TypeOf(asset) {
@@ -234,12 +269,10 @@ func (s *StoreImpl) Purge(excluded []WritableAsset) error {
 	}
 
 	for _, asset := range toPurge {
-		logrus.Debugf("Purging asset %q", asset.Name())
-		for _, f := range asset.Files() {
-			if err := os.Remove(filepath.Join(s.directory, f.Filename)); err != nil {
-				return errors.Wrapf(err, "failed to remove file %q", f.Filename)
-			}
+		if err := deleteAssetFromDisk(asset, s.directory); err != nil {
+			return err
 		}
 	}
+	s.markedForPurge = []WritableAsset{}
 	return nil
 }