Merge pull request #9474 from rdrgmnzs/fix-non-standard-backupStore-v2

Fix where etcd-cluster-spec is writen when etcd's BackupStore is defined -v2

pkg/apis/kops/validation/validation.go

@@ -159,6 +159,7 @@ func validateClusterSpec(spec *kops.ClusterSpec, c *kops.Cluster, fieldPath *fie
 			for i, etcdCluster := range spec.EtcdClusters {
 				allErrs = append(allErrs, validateEtcdClusterSpec(etcdCluster, c, fieldEtcdClusters.Index(i))...)
 			}
+			allErrs = append(allErrs, validateEtcdBackupStore(spec.EtcdClusters, fieldEtcdClusters)...)
 			allErrs = append(allErrs, validateEtcdTLS(spec.EtcdClusters, fieldEtcdClusters)...)
 			allErrs = append(allErrs, validateEtcdStorage(spec.EtcdClusters, fieldEtcdClusters)...)
 		}
@@ -796,6 +797,20 @@ func validateEtcdClusterSpec(spec *kops.EtcdClusterSpec, c *kops.Cluster, fieldP
 	return allErrs
 }
 
+// validateEtcdBackupStore checks that the etcd clusters backupStore path is unique.
+func validateEtcdBackupStore(specs []*kops.EtcdClusterSpec, fieldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	etcdBackupStore := make(map[string]bool)
+	for _, x := range specs {
+		if _, alreadyUsed := etcdBackupStore[x.Name]; alreadyUsed {
+			allErrs = append(allErrs, field.Forbidden(fieldPath.Index(0).Child("backupStore"), "the backup store must be unique for each etcd cluster"))
+		}
+		etcdBackupStore[x.Name] = true
+	}
+
+	return allErrs
+}
+
 // validateEtcdTLS checks the TLS settings for etcd are valid
 func validateEtcdTLS(specs []*kops.EtcdClusterSpec, fieldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}

pkg/model/components/etcdmanager/model.go

@@ -97,11 +97,19 @@ func (b *EtcdManagerBuilder) Build(c *fi.ModelBuilderContext) error {
 			return err
 		}
 
+		// Ensure a unique backup location for each etcd cluster
+		// if a backupStore is not specified.
+		var location string
+		if backupStore == "" {
+			location = "backups/etcd/" + etcdCluster.Name
+		}
+
 		c.AddTask(&fitasks.ManagedFile{
 			Contents:  fi.WrapResource(fi.NewBytesResource(d)),
 			Lifecycle: b.Lifecycle,
+			Base:      fi.String(backupStore),
 			// TODO: We need this to match the backup base (currently)
-			Location: fi.String("backups/etcd/" + etcdCluster.Name + "/control/etcd-cluster-spec"),
+			Location: fi.String(location + "/control/etcd-cluster-spec"),
 			Name:     fi.String("etcd-cluster-spec-" + name),
 		})
 

pkg/model/components/etcdmanager/tests/minimal/tasks.yaml

@@ -43,26 +43,29 @@ oldFormat: false
 subject: cn=etcd-peers-ca-main
 type: ca
 ---
+Base: memfs://clusters.example.com/minimal.example.com/backups/etcd-events
 Contents:
   Name: ""
   Resource: |-
     {
       "memberCount": 1
     }
 Lifecycle: null
-Location: backups/etcd/events/control/etcd-cluster-spec
+Location: /control/etcd-cluster-spec
 Name: etcd-cluster-spec-events
 ---
+Base: memfs://clusters.example.com/minimal.example.com/backups/etcd-main
 Contents:
   Name: ""
   Resource: |-
     {
       "memberCount": 1
     }
 Lifecycle: null
-Location: backups/etcd/main/control/etcd-cluster-spec
+Location: /control/etcd-cluster-spec
 Name: etcd-cluster-spec-main
 ---
+Base: null
 Contents:
   Name: ""
   Resource: |
@@ -128,6 +131,7 @@ Lifecycle: null
 Location: manifests/etcd/events.yaml
 Name: manifests-etcdmanager-events
 ---
+Base: null
 Contents:
   Name: ""
   Resource: |

pkg/model/components/etcdmanager/tests/old_versions_mount_hosts/tasks.yaml

@@ -43,26 +43,29 @@ oldFormat: false
 subject: cn=etcd-peers-ca-main
 type: ca
 ---
+Base: memfs://clusters.example.com/minimal.example.com/backups/etcd-events
 Contents:
   Name: ""
   Resource: |-
     {
       "memberCount": 1
     }
 Lifecycle: null
-Location: backups/etcd/events/control/etcd-cluster-spec
+Location: /control/etcd-cluster-spec
 Name: etcd-cluster-spec-events
 ---
+Base: memfs://clusters.example.com/minimal.example.com/backups/etcd-main
 Contents:
   Name: ""
   Resource: |-
     {
       "memberCount": 1
     }
 Lifecycle: null
-Location: backups/etcd/main/control/etcd-cluster-spec
+Location: /control/etcd-cluster-spec
 Name: etcd-cluster-spec-main
 ---
+Base: null
 Contents:
   Name: ""
   Resource: |
@@ -134,6 +137,7 @@ Lifecycle: null
 Location: manifests/etcd/events.yaml
 Name: manifests-etcdmanager-events
 ---
+Base: null
 Contents:
   Name: ""
   Resource: |

pkg/model/components/etcdmanager/tests/overwrite_settings/tasks.yaml

@@ -43,26 +43,29 @@ oldFormat: false
 subject: cn=etcd-peers-ca-main
 type: ca
 ---
+Base: memfs://clusters.example.com/minimal.example.com/backups/etcd-events
 Contents:
   Name: ""
   Resource: |-
     {
       "memberCount": 1
     }
 Lifecycle: null
-Location: backups/etcd/events/control/etcd-cluster-spec
+Location: /control/etcd-cluster-spec
 Name: etcd-cluster-spec-events
 ---
+Base: memfs://clusters.example.com/minimal.example.com/backups/etcd-main
 Contents:
   Name: ""
   Resource: |-
     {
       "memberCount": 1
     }
 Lifecycle: null
-Location: backups/etcd/main/control/etcd-cluster-spec
+Location: /control/etcd-cluster-spec
 Name: etcd-cluster-spec-main
 ---
+Base: null
 Contents:
   Name: ""
   Resource: |
@@ -131,6 +134,7 @@ Lifecycle: null
 Location: manifests/etcd/events.yaml
 Name: manifests-etcdmanager-events
 ---
+Base: null
 Contents:
   Name: ""
   Resource: |

pkg/model/components/etcdmanager/tests/proxy/tasks.yaml

@@ -43,26 +43,29 @@ oldFormat: false
 subject: cn=etcd-peers-ca-main
 type: ca
 ---
+Base: memfs://clusters.example.com/minimal.example.com/backups/etcd-events
 Contents:
   Name: ""
   Resource: |-
     {
       "memberCount": 1
     }
 Lifecycle: null
-Location: backups/etcd/events/control/etcd-cluster-spec
+Location: /control/etcd-cluster-spec
 Name: etcd-cluster-spec-events
 ---
+Base: memfs://clusters.example.com/minimal.example.com/backups/etcd-main
 Contents:
   Name: ""
   Resource: |-
     {
       "memberCount": 1
     }
 Lifecycle: null
-Location: backups/etcd/main/control/etcd-cluster-spec
+Location: /control/etcd-cluster-spec
 Name: etcd-cluster-spec-main
 ---
+Base: null
 Contents:
   Name: ""
   Resource: |
@@ -143,6 +146,7 @@ Lifecycle: null
 Location: manifests/etcd/events.yaml
 Name: manifests-etcdmanager-events
 ---
+Base: null
 Contents:
   Name: ""
   Resource: |

pkg/model/components/kubeapiserver/tests/minimal/tasks.yaml

@@ -1,3 +1,4 @@
+Base: null
 Contents:
   Name: ""
   Resource: |

upup/pkg/fi/fitasks/managedfile.go

@@ -24,19 +24,24 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/kops/pkg/acls"
 	"k8s.io/kops/upup/pkg/fi"
+	"k8s.io/kops/util/pkg/vfs"
 )
 
 //go:generate fitask -type=ManagedFile
 type ManagedFile struct {
 	Name      *string
 	Lifecycle *fi.Lifecycle
 
+	Base     *string
 	Location *string
 	Contents *fi.ResourceHolder
 }
 
 func (e *ManagedFile) Find(c *fi.Context) (*ManagedFile, error) {
-	managedFiles := c.ClusterConfigBase
+	managedFiles, err := getBasePath(c, e)
+	if err != nil {
+		return nil, err
+	}
 
 	location := fi.StringValue(e.Location)
 	if location == "" {
@@ -53,6 +58,7 @@ func (e *ManagedFile) Find(c *fi.Context) (*ManagedFile, error) {
 
 	actual := &ManagedFile{
 		Name:     e.Name,
+		Base:     e.Base,
 		Location: e.Location,
 		Contents: fi.WrapResource(fi.NewBytesResource(existingData)),
 	}
@@ -90,7 +96,11 @@ func (_ *ManagedFile) Render(c *fi.Context, a, e, changes *ManagedFile) error {
 		return fmt.Errorf("error reading contents of ManagedFile: %v", err)
 	}
 
-	p := c.ClusterConfigBase.Join(location)
+	p, err := getBasePath(c, e)
+	if err != nil {
+		return err
+	}
+	p = p.Join(location)
 
 	acl, err := acls.GetACL(p, c.Cluster)
 	if err != nil {
@@ -104,3 +114,16 @@ func (_ *ManagedFile) Render(c *fi.Context, a, e, changes *ManagedFile) error {
 
 	return nil
 }
+
+func getBasePath(c *fi.Context, e *ManagedFile) (vfs.Path, error) {
+	base := fi.StringValue(e.Base)
+	if base != "" {
+		p, err := vfs.Context.BuildVfsPath(base)
+		if err != nil {
+			return nil, fmt.Errorf("error parsing ManagedFile Base %q: %v", base, err)
+		}
+		return p, nil
+	}
+
+	return c.ClusterConfigBase, nil
+}