Merge pull request #9213 from johngmyers/refactor-update

Refactor Debian automatic upgrades to Go code

nodeup/pkg/model/update_service.go

@@ -26,41 +26,45 @@ import (
 	"k8s.io/klog"
 )
 
-// UpdateServiceBuilder disables the OS automatic updates
+// UpdateServiceBuilder enables/disables the OS automatic updates.
 type UpdateServiceBuilder struct {
 	*NodeupModelContext
 }
 
-// ServiceName is the name given to the service to be created
-const ServiceName = "update-service"
+const flatcarServiceName = "update-service"
+const debianPackageName = "unattended-upgrades"
 
 var _ fi.ModelBuilder = &UpdateServiceBuilder{}
 
-// Build is responsible for creating the relevant systemd service based on OS
+// Build is responsible for configuring automatic updates based on the OS.
 func (b *UpdateServiceBuilder) Build(c *fi.ModelBuilderContext) error {
+
+	if b.Distribution == distros.DistributionFlatcar {
+		b.buildFlatcarSystemdService(c)
+	} else if b.Distribution.IsDebianFamily() {
+		b.buildDebianPackage(c)
+	}
+
+	return nil
+}
+
+func (b *UpdateServiceBuilder) buildFlatcarSystemdService(c *fi.ModelBuilderContext) {
 	if b.Cluster.Spec.UpdatePolicy == nil || *b.Cluster.Spec.UpdatePolicy != kops.UpdatePolicyExternal {
-		klog.Infof("UpdatePolicy not set in Cluster Spec; skipping creation of %s", ServiceName)
-		return nil
+		klog.Infof("UpdatePolicy not set in Cluster Spec; skipping creation of %s", flatcarServiceName)
+		return
 	}
 
 	for _, spec := range [][]kops.HookSpec{b.InstanceGroup.Spec.Hooks, b.Cluster.Spec.Hooks} {
 		for _, hook := range spec {
-			if hook.Name == ServiceName || hook.Name == ServiceName+".service" {
-				klog.Infof("Detected kops Hook for '%s'; skipping creation", ServiceName)
-				return nil
+			if hook.Name == flatcarServiceName || hook.Name == flatcarServiceName+".service" {
+				klog.Infof("Detected kops Hook for '%s'; skipping creation", flatcarServiceName)
+				return
 			}
 		}
 	}
 
-	if b.Distribution == distros.DistributionFlatcar {
-		klog.Infof("Detected OS %s; building %s service to disable update scheduler", ServiceName, b.Distribution)
-		c.AddTask(b.buildFlatcarSystemdService())
-	}
-
-	return nil
-}
+	klog.Infof("Detected OS %s; building %s service to disable update scheduler", b.Distribution, flatcarServiceName)
 
-func (b *UpdateServiceBuilder) buildFlatcarSystemdService() *nodetasks.Service {
 	manifest := &systemd.Manifest{}
 	manifest.Set("Unit", "Description", "Disable OS Update Scheduler")
 
@@ -69,14 +73,35 @@ func (b *UpdateServiceBuilder) buildFlatcarSystemdService() *nodetasks.Service {
 	manifest.Set("Service", "ExecStart", "/usr/bin/systemctl mask --now locksmithd.service")
 
 	manifestString := manifest.Render()
-	klog.V(8).Infof("Built service manifest %q\n%s", ServiceName, manifestString)
+	klog.V(8).Infof("Built service manifest %q\n%s", flatcarServiceName, manifestString)
 
 	service := &nodetasks.Service{
-		Name:       ServiceName + ".service",
+		Name:       flatcarServiceName + ".service",
 		Definition: s(manifestString),
 	}
 
 	service.InitDefaults()
+	c.AddTask(service)
+}
+
+func (b *UpdateServiceBuilder) buildDebianPackage(c *fi.ModelBuilderContext) {
+	if b.Cluster.Spec.UpdatePolicy != nil && *b.Cluster.Spec.UpdatePolicy == kops.UpdatePolicyExternal {
+		klog.Infof("UpdatePolicy is External; skipping installation of %s", debianPackageName)
+		return
+	}
+
+	klog.Infof("Detected OS %s; installing %s package", b.Distribution, debianPackageName)
+
+	contents := `APT::Periodic::Update-Package-Lists "1";
+APT::Periodic::Unattended-Upgrade "1";
+
+APT::Periodic::AutocleanInterval "7";
+`
+	c.AddTask(&nodetasks.File{
+		Path:     "/etc/apt/apt.conf.d/20auto-upgrades",
+		Contents: fi.NewStringResource(contents),
+		Type:     nodetasks.FileType_File,
+	})
 
-	return service
+	c.AddTask(&nodetasks.Package{Name: debianPackageName})
 }

upup/pkg/fi/nodeup/loader.go

@@ -96,8 +96,7 @@ func (l *Loader) Build(baseDir vfs.Path) (map[string]fi.Task, error) {
 	tw := &loader.TreeWalker{
 		DefaultHandler: ignoreHandler,
 		Contexts: map[string]loader.Handler{
-			"files":    ignoreHandler,
-			"packages": ignoreHandler,
+			"files": ignoreHandler,
 		},
 		Tags: l.tags,
 	}
@@ -111,8 +110,7 @@ func (l *Loader) Build(baseDir vfs.Path) (map[string]fi.Task, error) {
 	tw = &loader.TreeWalker{
 		DefaultHandler: l.handleFile,
 		Contexts: map[string]loader.Handler{
-			"files":    l.handleFile,
-			"packages": l.newTaskHandler("package/", nodetasks.NewPackage),
+			"files": l.handleFile,
 		},
 		Tags: l.tags,
 	}
@@ -150,36 +148,6 @@ func (l *Loader) Build(baseDir vfs.Path) (map[string]fi.Task, error) {
 
 type TaskBuilder func(name string, contents string, meta string) (fi.Task, error)
 
-func (l *Loader) newTaskHandler(prefix string, builder TaskBuilder) loader.Handler {
-	return func(i *loader.TreeWalkItem) error {
-		contents, err := i.ReadString()
-		if err != nil {
-			return err
-		}
-		name := i.Name
-		if strings.HasSuffix(name, ".template") {
-			name = strings.TrimSuffix(name, ".template")
-			expanded, err := l.executeTemplate(name, contents)
-			if err != nil {
-				return fmt.Errorf("error executing template %q: %v", i.RelativePath, err)
-			}
-
-			contents = expanded
-		}
-
-		task, err := builder(name, contents, i.Meta)
-		if err != nil {
-			return fmt.Errorf("error building %s for %q: %v", i.Name, i.Path, err)
-		}
-		key := prefix + i.RelativePath
-
-		if task != nil {
-			l.tasks[key] = task
-		}
-		return nil
-	}
-}
-
 func (l *Loader) handleFile(i *loader.TreeWalkItem) error {
 	var task *nodetasks.File
 	defaultFileType := nodetasks.FileType_File

upup/pkg/fi/nodeup/nodetasks/package.go

@@ -17,7 +17,6 @@ limitations under the License.
 package nodetasks
 
 import (
-	"encoding/json"
 	"fmt"
 	"os"
 	"os/exec"
@@ -132,23 +131,6 @@ func (p *Package) String() string {
 	return fmt.Sprintf("Package: %s", p.Name)
 }
 
-func NewPackage(name string, contents string, meta string) (fi.Task, error) {
-	p := &Package{Name: name}
-	if contents != "" {
-		err := json.Unmarshal([]byte(contents), p)
-		if err != nil {
-			return nil, fmt.Errorf("error parsing json for package %q: %v", name, err)
-		}
-	}
-
-	// Default values: we want to install a package so that it is healthy
-	if p.Healthy == nil {
-		p.Healthy = fi.Bool(true)
-	}
-
-	return p, nil
-}
-
 func (e *Package) Find(c *fi.Context) (*Package, error) {
 	target := c.Target.(*local.LocalTarget)