From 76fc186060ed3e4c54191050d1f22084c5aa3224 Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Mon, 19 Dec 2022 23:09:33 -0800 Subject: [PATCH] Remove nodeup's unused cloudinit target --- cmd/nodeup/main.go | 2 +- .../fi/nodeup/cloudinit/cloud_init_target.go | 184 ------------------ upup/pkg/fi/nodeup/command.go | 4 - upup/pkg/fi/nodeup/nodetasks/archive.go | 19 -- upup/pkg/fi/nodeup/nodetasks/bindmount.go | 5 - upup/pkg/fi/nodeup/nodetasks/chattr.go | 5 - upup/pkg/fi/nodeup/nodetasks/file.go | 35 ---- upup/pkg/fi/nodeup/nodetasks/group.go | 11 -- upup/pkg/fi/nodeup/nodetasks/load_image.go | 5 - upup/pkg/fi/nodeup/nodetasks/package.go | 22 --- upup/pkg/fi/nodeup/nodetasks/service.go | 23 --- .../nodeup/nodetasks/update_etc_hosts_task.go | 5 - .../fi/nodeup/nodetasks/update_packages.go | 6 - upup/pkg/fi/nodeup/nodetasks/user.go | 11 -- 14 files changed, 1 insertion(+), 336 deletions(-) delete mode 100644 upup/pkg/fi/nodeup/cloudinit/cloud_init_target.go diff --git a/cmd/nodeup/main.go b/cmd/nodeup/main.go index d65a418b03..8706128956 100644 --- a/cmd/nodeup/main.go +++ b/cmd/nodeup/main.go @@ -49,7 +49,7 @@ func main() { flag.StringVar(&flagCacheDir, "cache", "/var/cache/nodeup", "the location for the local asset cache") flag.IntVar(&flagRetries, "retries", -1, "maximum number of retries on failure: -1 means retry forever") flag.BoolVar(&dryrun, "dryrun", false, "Don't create cloud resources; just show what would be done") - flag.StringVar(&target, "target", target, "Target - direct, cloudinit") + flag.StringVar(&target, "target", target, "Target - direct, dryrun") flag.BoolVar(&installSystemdUnit, "install-systemd-unit", installSystemdUnit, "If true, will install a systemd unit instead of running directly") if dryrun { diff --git a/upup/pkg/fi/nodeup/cloudinit/cloud_init_target.go b/upup/pkg/fi/nodeup/cloudinit/cloud_init_target.go deleted file mode 100644 index 3e4d313fce..0000000000 --- a/upup/pkg/fi/nodeup/cloudinit/cloud_init_target.go +++ /dev/null @@ -1,184 +0,0 @@ -/* -Copyright 2019 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package cloudinit - -import ( - "encoding/base64" - "fmt" - "io" - "os" - "path" - - "k8s.io/klog/v2" - "k8s.io/kops/upup/pkg/fi" - "k8s.io/kops/upup/pkg/fi/utils" -) - -type CloudInitTarget struct { - Config *CloudConfig - out io.Writer -} - -type AddBehaviour int - -const ( - Always AddBehaviour = iota - Once -) - -func NewCloudInitTarget(out io.Writer) *CloudInitTarget { - t := &CloudInitTarget{ - Config: &CloudConfig{}, - out: out, - } - return t -} - -var _ fi.NodeupTarget = &CloudInitTarget{} - -type CloudConfig struct { - PackageUpdate bool `json:"package_update"` - - Packages []string `json:"packages,omitempty"` - RunCommmands [][]string `json:"runcmd,omitempty"` - WriteFiles []*CloudConfigFile `json:"write_files,omitempty"` -} - -type CloudConfigFile struct { - Encoding string `json:"encoding,omitempty"` - Owner string `json:"owner,omitempty"` - Path string `json:"path,omitempty"` - Permissions string `json:"permissions,omitempty"` - Content string `json:"content,omitempty"` -} - -func (t *CloudInitTarget) ProcessDeletions() bool { - // We don't expect any, but it would be our job to process them - return true -} - -func (t *CloudInitTarget) AddMkdirpCommand(p string, dirMode os.FileMode) { - t.AddCommand(Once, "mkdir", "-p", "-m", fi.FileModeToString(dirMode), p) -} - -func (t *CloudInitTarget) AddDownloadCommand(addBehaviour AddBehaviour, url string, dest string) { - // TODO: Create helper to download reliably and validate hash? - // ... but then why not just use cloudup :-) - t.AddCommand(addBehaviour, "curl", "-f", "--ipv4", "-Lo", dest, "--connect-timeout", "20", "--retry", "6", "--retry-delay", "10", url) -} - -func (t *CloudInitTarget) fetch(p *fi.Source, destPath string) { - // We could probably move this to fi.Source - it is likely to be the same for every provider - if p.URL != "" { - if p.Parent != nil { - klog.Fatalf("unexpected parent with SourceURL in FetchInstructions: %v", p) - } - t.AddDownloadCommand(Once, p.URL, destPath) - } else if p.ExtractFromArchive != "" { - if p.Parent == nil { - klog.Fatalf("unexpected ExtractFromArchive without parent in FetchInstructions: %v", p) - } - - // TODO: Remove duplicate commands? - archivePath := "/tmp/" + utils.SanitizeString(p.Parent.Key()) - t.fetch(p.Parent, archivePath) - - extractDir := "/tmp/extracted_" + utils.SanitizeString(p.Parent.Key()) - t.AddMkdirpCommand(extractDir, 0o755) - t.AddCommand(Once, "tar", "zxf", archivePath, "-C", extractDir) - - // Always because this shouldn't happen and we want an indication that it happened - t.AddCommand(Always, "cp", path.Join(extractDir, p.ExtractFromArchive), destPath) - } else { - klog.Fatalf("unknown FetchInstructions: %v", p) - } -} - -func (t *CloudInitTarget) WriteFile(destPath string, contents fi.Resource, fileMode os.FileMode, dirMode os.FileMode) error { - var p *fi.Source - - if hs, ok := contents.(fi.HasSource); ok { - p = hs.GetSource() - } - - if p != nil { - t.AddMkdirpCommand(path.Dir(destPath), dirMode) - t.fetch(p, destPath) - } else { - // TODO: No way to specify parent dir permissions? - f := &CloudConfigFile{ - Encoding: "b64", - Owner: "root:root", - Permissions: fi.FileModeToString(fileMode), - Path: destPath, - } - - d, err := fi.ResourceAsBytes(contents) - if err != nil { - return err - } - - // Not a strict limit, just a sanity check - if len(d) > 256*1024 { - return fmt.Errorf("resource is very large (failed sanity-check): %v", contents) - } - - f.Content = base64.StdEncoding.EncodeToString(d) - - t.Config.WriteFiles = append(t.Config.WriteFiles, f) - } - return nil -} - -func (t *CloudInitTarget) Chown(path string, user, group string) { - t.AddCommand(Always, "chown", user+":"+group, path) -} - -func (t *CloudInitTarget) AddCommand(addBehaviour AddBehaviour, args ...string) { - switch addBehaviour { - case Always: - break - - case Once: - for _, c := range t.Config.RunCommmands { - if utils.StringSlicesEqual(args, c) { - klog.V(2).Infof("skipping pre-existing command because AddBehaviour=Once: %q", args) - return - } - } - - default: - klog.Fatalf("unknown AddBehaviour: %v", addBehaviour) - } - - t.Config.RunCommmands = append(t.Config.RunCommmands, args) -} - -func (t *CloudInitTarget) Finish(taskMap map[string]fi.NodeupTask) error { - d, err := utils.YamlMarshal(t.Config) - if err != nil { - return fmt.Errorf("error serializing config to yaml: %v", err) - } - - conf := "#cloud-config\n" + string(d) - - _, err = t.out.Write([]byte(conf)) - if err != nil { - return fmt.Errorf("error writing cloud-init data to output: %v", err) - } - return nil -} diff --git a/upup/pkg/fi/nodeup/command.go b/upup/pkg/fi/nodeup/command.go index ee1a960bf7..fba5ee004d 100644 --- a/upup/pkg/fi/nodeup/command.go +++ b/upup/pkg/fi/nodeup/command.go @@ -54,7 +54,6 @@ import ( "k8s.io/kops/upup/pkg/fi/cloudup/gce/gcediscovery" "k8s.io/kops/upup/pkg/fi/cloudup/gce/tpm/gcetpmsigner" "k8s.io/kops/upup/pkg/fi/cloudup/hetzner" - "k8s.io/kops/upup/pkg/fi/nodeup/cloudinit" "k8s.io/kops/upup/pkg/fi/nodeup/local" "k8s.io/kops/upup/pkg/fi/nodeup/nodetasks" "k8s.io/kops/upup/pkg/fi/secrets" @@ -378,9 +377,6 @@ func (c *NodeUpCommand) Run(out io.Writer) error { case "dryrun": assetBuilder := assets.NewAssetBuilder(c.cluster, false) target = fi.NewNodeupDryRunTarget(assetBuilder, out) - case "cloudinit": - checkExisting = false - target = cloudinit.NewCloudInitTarget(out) default: return fmt.Errorf("unsupported target type %q", c.Target) } diff --git a/upup/pkg/fi/nodeup/nodetasks/archive.go b/upup/pkg/fi/nodeup/nodetasks/archive.go index 30b530c252..0c4d58799c 100644 --- a/upup/pkg/fi/nodeup/nodetasks/archive.go +++ b/upup/pkg/fi/nodeup/nodetasks/archive.go @@ -29,7 +29,6 @@ import ( "k8s.io/klog/v2" "k8s.io/kops/upup/pkg/fi" - "k8s.io/kops/upup/pkg/fi/nodeup/cloudinit" "k8s.io/kops/upup/pkg/fi/nodeup/local" "k8s.io/kops/util/pkg/hashing" ) @@ -211,21 +210,3 @@ func (_ *Archive) RenderLocal(t *local.LocalTarget, a, e, changes *Archive) erro return nil } - -// RenderCloudInit implements fi.Task::Render functionality for a CloudInit target -func (_ *Archive) RenderCloudInit(t *cloudinit.CloudInitTarget, a, e, changes *Archive) error { - archiveName := e.Name - - localFile := path.Join(localArchiveDir, archiveName) - t.AddMkdirpCommand(localArchiveDir, 0o755) - - targetDir := e.TargetDir - t.AddMkdirpCommand(targetDir, 0o755) - - url := e.Source - t.AddDownloadCommand(cloudinit.Always, url, localFile) - - t.AddCommand(cloudinit.Always, "tar", "xf", localFile, "-C", targetDir) - - return nil -} diff --git a/upup/pkg/fi/nodeup/nodetasks/bindmount.go b/upup/pkg/fi/nodeup/nodetasks/bindmount.go index 292ce9b5d2..dbcfae9b27 100644 --- a/upup/pkg/fi/nodeup/nodetasks/bindmount.go +++ b/upup/pkg/fi/nodeup/nodetasks/bindmount.go @@ -24,7 +24,6 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog/v2" "k8s.io/kops/upup/pkg/fi" - "k8s.io/kops/upup/pkg/fi/nodeup/cloudinit" "k8s.io/kops/upup/pkg/fi/nodeup/local" ) @@ -249,7 +248,3 @@ func (e *BindMount) execute(t Executor) error { return nil } - -func (_ *BindMount) RenderCloudInit(t *cloudinit.CloudInitTarget, a, e, changes *BindMount) error { - return fmt.Errorf("BindMount::RenderCloudInit not implemented") -} diff --git a/upup/pkg/fi/nodeup/nodetasks/chattr.go b/upup/pkg/fi/nodeup/nodetasks/chattr.go index 1bd8774503..6a1075a971 100644 --- a/upup/pkg/fi/nodeup/nodetasks/chattr.go +++ b/upup/pkg/fi/nodeup/nodetasks/chattr.go @@ -22,7 +22,6 @@ import ( "k8s.io/klog/v2" "k8s.io/kops/upup/pkg/fi" - "k8s.io/kops/upup/pkg/fi/nodeup/cloudinit" "k8s.io/kops/upup/pkg/fi/nodeup/local" ) @@ -80,7 +79,3 @@ func (e *Chattr) execute(t Executor) error { return nil } - -func (_ *Chattr) RenderCloudInit(t *cloudinit.CloudInitTarget, a, e, changes *Chattr) error { - return fmt.Errorf("Chattr::RenderCloudInit not implemented") -} diff --git a/upup/pkg/fi/nodeup/nodetasks/file.go b/upup/pkg/fi/nodeup/nodetasks/file.go index e7df016fbc..4d1fcf926b 100644 --- a/upup/pkg/fi/nodeup/nodetasks/file.go +++ b/upup/pkg/fi/nodeup/nodetasks/file.go @@ -27,7 +27,6 @@ import ( "k8s.io/klog/v2" "k8s.io/kops/upup/pkg/fi" - "k8s.io/kops/upup/pkg/fi/nodeup/cloudinit" "k8s.io/kops/upup/pkg/fi/nodeup/install" "k8s.io/kops/upup/pkg/fi/nodeup/local" ) @@ -313,37 +312,3 @@ func (_ *File) RenderLocal(_ *local.LocalTarget, a, e, changes *File) error { return nil } - -func (_ *File) RenderCloudInit(t *cloudinit.CloudInitTarget, a, e, changes *File) error { - dirMode := os.FileMode(0o755) - fileMode, err := fi.ParseFileMode(fi.ValueOf(e.Mode), 0o644) - if err != nil { - return fmt.Errorf("invalid file mode for %s: %q", e.Path, *e.Mode) - } - - if e.Type == FileType_Symlink { - t.AddCommand(cloudinit.Always, "ln", "-s", fi.ValueOf(e.Symlink), e.Path) - } else if e.Type == FileType_Directory { - parent := filepath.Dir(strings.TrimSuffix(e.Path, "/")) - t.AddCommand(cloudinit.Once, "mkdir", "-p", "-m", fi.FileModeToString(dirMode), parent) - t.AddCommand(cloudinit.Once, "mkdir", "-m", fi.FileModeToString(dirMode), e.Path) - } else if e.Type == FileType_File { - err = t.WriteFile(e.Path, e.Contents, fileMode, dirMode) - if err != nil { - return err - } - } else { - return fmt.Errorf("File type=%q not valid/supported", e.Type) - } - - if e.Owner != nil || e.Group != nil { - t.Chown(e.Path, fi.ValueOf(e.Owner), fi.ValueOf(e.Group)) - } - - if e.OnChangeExecute != nil { - return fmt.Errorf("OnChangeExecute not supported with CloudInit") - // t.AddCommand(cloudinit.Always, e.OnChangeExecute...) - } - - return nil -} diff --git a/upup/pkg/fi/nodeup/nodetasks/group.go b/upup/pkg/fi/nodeup/nodetasks/group.go index 6c53fef5e9..c4a57e9146 100644 --- a/upup/pkg/fi/nodeup/nodetasks/group.go +++ b/upup/pkg/fi/nodeup/nodetasks/group.go @@ -24,7 +24,6 @@ import ( "k8s.io/klog/v2" "k8s.io/kops/upup/pkg/fi" - "k8s.io/kops/upup/pkg/fi/nodeup/cloudinit" "k8s.io/kops/upup/pkg/fi/nodeup/local" ) @@ -119,13 +118,3 @@ func (_ *GroupTask) RenderLocal(t *local.LocalTarget, a, e, changes *GroupTask) return nil } - -func (_ *GroupTask) RenderCloudInit(t *cloudinit.CloudInitTarget, a, e, changes *GroupTask) error { - args := buildGroupaddArgs(e) - cmd := []string{"groupadd"} - cmd = append(cmd, args...) - klog.Infof("Creating group %q", e.Name) - t.AddCommand(cloudinit.Once, cmd...) - - return nil -} diff --git a/upup/pkg/fi/nodeup/nodetasks/load_image.go b/upup/pkg/fi/nodeup/nodetasks/load_image.go index 4456631b22..bb23228438 100644 --- a/upup/pkg/fi/nodeup/nodetasks/load_image.go +++ b/upup/pkg/fi/nodeup/nodetasks/load_image.go @@ -27,7 +27,6 @@ import ( "k8s.io/klog/v2" "k8s.io/kops/pkg/backoff" "k8s.io/kops/upup/pkg/fi" - "k8s.io/kops/upup/pkg/fi/nodeup/cloudinit" "k8s.io/kops/upup/pkg/fi/nodeup/local" "k8s.io/kops/upup/pkg/fi/utils" "k8s.io/kops/util/pkg/hashing" @@ -168,7 +167,3 @@ func (_ *LoadImageTask) RenderLocal(t *local.LocalTarget, a, e, changes *LoadIma return nil } - -func (_ *LoadImageTask) RenderCloudInit(t *cloudinit.CloudInitTarget, a, e, changes *LoadImageTask) error { - return fmt.Errorf("LoadImageTask::RenderCloudInit not implemented") -} diff --git a/upup/pkg/fi/nodeup/nodetasks/package.go b/upup/pkg/fi/nodeup/nodetasks/package.go index 7e57efe318..1bb5e6a946 100644 --- a/upup/pkg/fi/nodeup/nodetasks/package.go +++ b/upup/pkg/fi/nodeup/nodetasks/package.go @@ -28,7 +28,6 @@ import ( "k8s.io/klog/v2" "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/upup/pkg/fi" - "k8s.io/kops/upup/pkg/fi/nodeup/cloudinit" "k8s.io/kops/upup/pkg/fi/nodeup/local" "k8s.io/kops/util/pkg/distributions" "k8s.io/kops/util/pkg/hashing" @@ -377,24 +376,3 @@ func (_ *Package) RenderLocal(t *local.LocalTarget, a, e, changes *Package) erro return nil } - -func (_ *Package) RenderCloudInit(t *cloudinit.CloudInitTarget, a, e, changes *Package) error { - packageName := e.Name - if e.Source != nil { - localFile := path.Join(localPackageDir, packageName) - t.AddMkdirpCommand(localPackageDir, 0o755) - - url := *e.Source - t.AddDownloadCommand(cloudinit.Always, url, localFile) - - t.AddCommand(cloudinit.Always, "dpkg", "-i", localFile) - } else { - packageSpec := packageName - if e.Version != nil { - packageSpec += " " + *e.Version - } - t.Config.Packages = append(t.Config.Packages, packageSpec) - } - - return nil -} diff --git a/upup/pkg/fi/nodeup/nodetasks/service.go b/upup/pkg/fi/nodeup/nodetasks/service.go index b6fdbc6282..32b3ec40b9 100644 --- a/upup/pkg/fi/nodeup/nodetasks/service.go +++ b/upup/pkg/fi/nodeup/nodetasks/service.go @@ -26,7 +26,6 @@ import ( "k8s.io/klog/v2" "k8s.io/kops/upup/pkg/fi" - "k8s.io/kops/upup/pkg/fi/nodeup/cloudinit" "k8s.io/kops/upup/pkg/fi/nodeup/install" "k8s.io/kops/upup/pkg/fi/nodeup/local" "k8s.io/kops/util/pkg/distributions" @@ -421,28 +420,6 @@ func (s *Service) RenderLocal(_ *local.LocalTarget, a, e, changes *Service) erro return nil } -func (_ *Service) RenderCloudInit(t *cloudinit.CloudInitTarget, a, e, changes *Service) error { - systemdSystemPath, err := e.systemdSystemPath() - if err != nil { - return err - } - - serviceName := e.Name - - servicePath := path.Join(systemdSystemPath, serviceName) - err = t.WriteFile(servicePath, fi.NewStringResource(*e.Definition), 0o644, 0o755) - if err != nil { - return err - } - - if fi.ValueOf(e.ManageState) { - t.AddCommand(cloudinit.Once, "systemctl", "daemon-reload") - t.AddCommand(cloudinit.Once, "systemctl", "start", "--no-block", serviceName) - } - - return nil -} - func (s *Service) GetName() *string { return &s.Name } diff --git a/upup/pkg/fi/nodeup/nodetasks/update_etc_hosts_task.go b/upup/pkg/fi/nodeup/nodetasks/update_etc_hosts_task.go index 28d86a00b8..8a684509ba 100644 --- a/upup/pkg/fi/nodeup/nodetasks/update_etc_hosts_task.go +++ b/upup/pkg/fi/nodeup/nodetasks/update_etc_hosts_task.go @@ -22,7 +22,6 @@ import ( "k8s.io/klog/v2" "k8s.io/kops/protokube/pkg/gossip/dns/hosts" "k8s.io/kops/upup/pkg/fi" - "k8s.io/kops/upup/pkg/fi/nodeup/cloudinit" "k8s.io/kops/upup/pkg/fi/nodeup/local" ) @@ -94,7 +93,3 @@ func (_ *UpdateEtcHostsTask) RenderLocal(t *local.LocalTarget, a, e, changes *Up } return nil } - -func (_ *UpdateEtcHostsTask) RenderCloudInit(t *cloudinit.CloudInitTarget, a, e, changes *UpdateEtcHostsTask) error { - return fmt.Errorf("UpdateEtcHostsTask::RenderCloudInit not supported") -} diff --git a/upup/pkg/fi/nodeup/nodetasks/update_packages.go b/upup/pkg/fi/nodeup/nodetasks/update_packages.go index 5c6c9bc43f..63dacc6332 100644 --- a/upup/pkg/fi/nodeup/nodetasks/update_packages.go +++ b/upup/pkg/fi/nodeup/nodetasks/update_packages.go @@ -24,7 +24,6 @@ import ( "k8s.io/klog/v2" "k8s.io/kops/upup/pkg/fi" - "k8s.io/kops/upup/pkg/fi/nodeup/cloudinit" "k8s.io/kops/upup/pkg/fi/nodeup/local" "k8s.io/kops/util/pkg/distributions" ) @@ -94,8 +93,3 @@ func (_ *UpdatePackages) RenderLocal(t *local.LocalTarget, a, e, changes *Update return nil } - -func (_ *UpdatePackages) RenderCloudInit(t *cloudinit.CloudInitTarget, a, e, changes *UpdatePackages) error { - t.Config.PackageUpdate = true - return nil -} diff --git a/upup/pkg/fi/nodeup/nodetasks/user.go b/upup/pkg/fi/nodeup/nodetasks/user.go index a47b0cf315..075969eda9 100644 --- a/upup/pkg/fi/nodeup/nodetasks/user.go +++ b/upup/pkg/fi/nodeup/nodetasks/user.go @@ -24,7 +24,6 @@ import ( "k8s.io/klog/v2" "k8s.io/kops/upup/pkg/fi" - "k8s.io/kops/upup/pkg/fi/nodeup/cloudinit" "k8s.io/kops/upup/pkg/fi/nodeup/local" ) @@ -128,13 +127,3 @@ func (_ *UserTask) RenderLocal(t *local.LocalTarget, a, e, changes *UserTask) er return nil } - -func (_ *UserTask) RenderCloudInit(t *cloudinit.CloudInitTarget, a, e, changes *UserTask) error { - args := buildUseraddArgs(e) - cmd := []string{"useradd"} - cmd = append(cmd, args...) - klog.Infof("Creating user %q", e.Name) - t.AddCommand(cloudinit.Once, cmd...) - - return nil -}