Implement a Golang cluster installer job that hides the template

ARCHITECTURE.md

@@ -133,12 +133,7 @@ output, with parallel execution of steps leading to interwoven log output:
 2018/10/08 05:29:15 Building machine-config-operator
 2018/10/08 05:29:15 Building machine-config-controller
 2018/10/08 05:31:47 Build machine-config-server succeeded after 2m28s
-2018/10/08 05:31:47 Tagging machine-config-server into stable
 2018/10/08 05:31:56 Build machine-config-operator succeeded after 2m37s
-2018/10/08 05:31:56 Tagging machine-config-operator into stable
 2018/10/08 05:31:56 Build machine-config-daemon succeeded after 2m37s
-2018/10/08 05:31:56 Tagging machine-config-daemon into stable
 2018/10/08 05:32:00 Build machine-config-controller succeeded after 2m41s
-2018/10/08 05:32:00 Tagging machine-config-controller into stable
-2018/10/08 05:32:00 All images ready
 ```

cmd/ci-operator/main.go

@@ -9,6 +9,7 @@ import (
 	"errors"
 	"flag"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"log"
 	"os"
@@ -182,6 +183,7 @@ type options struct {
 	verbose bool
 	help    bool
 	dry     bool
+	print   bool
 
 	writeParams string
 	artifactDir string
@@ -219,6 +221,7 @@ func bindOptions(flag *flag.FlagSet) *options {
 	flag.StringVar(&opt.configSpecPath, "config", "", "The configuration file. If not specified the CONFIG_SPEC environment variable will be used.")
 	flag.Var(&opt.targets, "target", "One or more targets in the configuration to build. Only steps that are required for this target will be run.")
 	flag.BoolVar(&opt.dry, "dry-run", opt.dry, "Print the steps that would be run and the objects that would be created without executing any steps")
+	flag.BoolVar(&opt.print, "print-graph", opt.print, "Print a directed graph of the build steps and exit. Intended for use with the golang digraph utility.")
 
 	// add to the graph of things we run or create
 	flag.Var(&opt.templatePaths, "template", "A set of paths to optional templates to add as stages to this job. Each template is expected to contain at least one restart=Never pod. Parameters are filled from environment or from the automatic parameters generated by the operator.")
@@ -399,6 +402,13 @@ func (o *options) Run() error {
 			return fmt.Errorf("unable to write metadata.json for build: %v", err)
 		}
 
+		if o.print {
+			if err := printDigraph(os.Stdout, buildSteps); err != nil {
+				return fmt.Errorf("could not print graph: %v", err)
+			}
+			return nil
+		}
+
 		// convert the full graph into the subset we must run
 		nodes, err := api.BuildPartialGraph(buildSteps, o.targets.values)
 		if err != nil {
@@ -665,9 +675,6 @@ func (o *options) initializeNamespace() error {
 		})
 	}
 
-	if len(o.secrets) > 0 {
-		log.Printf("Populating secrets for test")
-	}
 	for _, secret := range o.secrets {
 		_, err := client.Secrets(o.namespace).Create(secret)
 		if kerrors.IsAlreadyExists(err) {
@@ -922,6 +929,22 @@ func topologicalSort(nodes []*api.StepNode) ([]*api.StepNode, error) {
 	return sortedNodes, nil
 }
 
+func printDigraph(w io.Writer, steps []api.Step) error {
+	for _, step := range steps {
+		for _, other := range steps {
+			if step == other {
+				continue
+			}
+			if api.HasAnyLinks(step.Requires(), other.Creates()) {
+				if _, err := fmt.Fprintf(w, "%s %s\n", step.Name(), other.Name()); err != nil {
+					return err
+				}
+			}
+		}
+	}
+	return nil
+}
+
 func printExecutionOrder(nodes []*api.StepNode) error {
 	ordered, err := topologicalSort(nodes)
 	if err != nil {

pkg/api/graph.go

@@ -242,7 +242,9 @@ func BuildPartialGraph(steps []Step, names []string) ([]*StepNode, error) {
 
 	var required []StepLink
 	candidates := make([]bool, len(steps))
+	var allNames []string
 	for i, step := range steps {
+		allNames = append(allNames, step.Name())
 		for j, name := range names {
 			if name != step.Name() {
 				continue
@@ -254,7 +256,7 @@ func BuildPartialGraph(steps []Step, names []string) ([]*StepNode, error) {
 		}
 	}
 	if len(names) > 0 {
-		return nil, fmt.Errorf("the following names were not found in the config or were duplicates: %s", strings.Join(names, ", "))
+		return nil, fmt.Errorf("the following names were not found in the config or were duplicates: %s (from %s)", strings.Join(names, ", "), strings.Join(allNames, ", "))
 	}
 
 	// identify all other steps that provide any links required by the current set

pkg/api/parameters.go

@@ -6,6 +6,48 @@ import (
 	"sync"
 )
 
+// Parameters allows a step to read values set by other steps.
+type Parameters interface {
+	Has(name string) bool
+	HasInput(name string) bool
+	Get(name string) (string, error)
+	Links(name string) []StepLink
+}
+
+type overrideParameters struct {
+	params    Parameters
+	overrides map[string]string
+}
+
+func (p *overrideParameters) Has(name string) bool {
+	if _, ok := p.overrides[name]; ok {
+		return true
+	}
+	return p.params.Has(name)
+}
+
+func (p *overrideParameters) HasInput(name string) bool {
+	return p.params.HasInput(name)
+}
+
+func (p *overrideParameters) Get(name string) (string, error) {
+	if value, ok := p.overrides[name]; ok {
+		return value, nil
+	}
+	return p.params.Get(name)
+}
+
+func (p *overrideParameters) Links(name string) []StepLink {
+	return p.params.Links(name)
+}
+
+func NewOverrideParameters(params Parameters, overrides map[string]string) Parameters {
+	return &overrideParameters{
+		params:    params,
+		overrides: overrides,
+	}
+}
+
 type DeferredParameters struct {
 	lock   sync.Mutex
 	fns    ParameterMap
@@ -61,6 +103,17 @@ func (p *DeferredParameters) Add(name string, link StepLink, fn func() (string,
 	}
 }
 
+// HasInput returns true if the named parameter is an input from outside the graph, rather
+// than provided either by the graph caller or another node.
+func (p *DeferredParameters) HasInput(name string) bool {
+	p.lock.Lock()
+	defer p.lock.Unlock()
+	_, ok := os.LookupEnv(name)
+	return ok
+}
+
+// Has returns true if the named parameter exists. Use HasInput if you need to know whether
+// the value comes from outside the graph.
 func (p *DeferredParameters) Has(name string) bool {
 	p.lock.Lock()
 	defer p.lock.Unlock()
@@ -75,20 +128,14 @@ func (p *DeferredParameters) Has(name string) bool {
 func (p *DeferredParameters) Links(name string) []StepLink {
 	p.lock.Lock()
 	defer p.lock.Unlock()
-	if _, ok := os.LookupEnv(name); ok {
-		return nil
-	}
 	return p.links[name]
 }
 
 func (p *DeferredParameters) AllLinks() []StepLink {
 	p.lock.Lock()
 	defer p.lock.Unlock()
 	var links []StepLink
-	for name, v := range p.links {
-		if _, ok := os.LookupEnv(name); ok {
-			continue
-		}
+	for _, v := range p.links {
 		links = append(links, v...)
 	}
 	return links

pkg/defaults/defaults.go

@@ -5,9 +5,10 @@ import (
 	"fmt"
 	"log"
 	"net/url"
-	"os"
 	"strings"
 
+	"github.com/openshift/ci-operator/pkg/steps/clusterinstall"
+
 	appsclientset "k8s.io/client-go/kubernetes/typed/apps/v1"
 	coreclientset "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/client-go/rest"
@@ -52,6 +53,7 @@ func FromConfig(
 	var templateClient steps.TemplateClient
 	var configMapGetter coreclientset.ConfigMapsGetter
 	var serviceGetter coreclientset.ServicesGetter
+	var secretGetter coreclientset.SecretsGetter
 	var podClient steps.PodClient
 
 	if clusterConfig != nil {
@@ -90,6 +92,7 @@ func FromConfig(
 		}
 		serviceGetter = coreGetter
 		configMapGetter = coreGetter
+		secretGetter = coreGetter
 
 		podClient = steps.NewPodClient(coreGetter, clusterConfig, coreGetter.RESTClient())
 	}
@@ -101,7 +104,7 @@ func FromConfig(
 	params.Add("NAMESPACE", nil, func() (string, error) { return jobSpec.Namespace, nil })
 
 	var imageStepLinks []api.StepLink
-	var releaseStep, initialReleaseStep api.Step
+	var hasReleaseStep bool
 	for _, rawStep := range stepConfigsForBuild(config, jobSpec) {
 		var step api.Step
 		var stepLinks []api.StepLink
@@ -141,20 +144,31 @@ func FromConfig(
 			step = release.ReleaseImagesTagStep(*rawStep.ReleaseImagesTagStepConfiguration, srcClient, imageClient, routeGetter, configMapGetter, params, jobSpec)
 			stepLinks = append(stepLinks, step.Creates()...)
 
-			releaseStep = release.AssembleReleaseStep(true, *rawStep.ReleaseImagesTagStepConfiguration, config.Resources, podClient, imageClient, artifactDir, jobSpec)
-			checkForFullyQualifiedStep(releaseStep, params)
+			hasReleaseStep = true
+
+			releaseStep := release.AssembleReleaseStep(true, *rawStep.ReleaseImagesTagStepConfiguration, params, config.Resources, podClient, imageClient, artifactDir, jobSpec)
+			addProvidesForStep(releaseStep, params)
+			buildSteps = append(buildSteps, releaseStep)
 
-			initialReleaseStep = release.AssembleReleaseStep(false, *rawStep.ReleaseImagesTagStepConfiguration, config.Resources, podClient, imageClient, artifactDir, jobSpec)
-			checkForFullyQualifiedStep(initialReleaseStep, params)
-			// initial release is always added
+			initialReleaseStep := release.AssembleReleaseStep(false, *rawStep.ReleaseImagesTagStepConfiguration, params, config.Resources, podClient, imageClient, artifactDir, jobSpec)
+			addProvidesForStep(initialReleaseStep, params)
 			buildSteps = append(buildSteps, initialReleaseStep)
 
+		} else if rawStep.TestStepConfiguration != nil && rawStep.TestStepConfiguration.OpenshiftInstallerClusterTestConfiguration != nil && rawStep.TestStepConfiguration.OpenshiftInstallerClusterTestConfiguration.Upgrade {
+			var err error
+			step, err = clusterinstall.E2ETestStep(*rawStep.TestStepConfiguration.OpenshiftInstallerClusterTestConfiguration, *rawStep.TestStepConfiguration, params, podClient, templateClient, secretGetter, artifactDir, jobSpec)
+			if err != nil {
+				return nil, nil, fmt.Errorf("unable to create end to end test step: %v", err)
+			}
+
 		} else if rawStep.TestStepConfiguration != nil {
 			step = steps.TestStep(*rawStep.TestStepConfiguration, config.Resources, podClient, artifactDir, jobSpec)
 		}
 
 		step, ok := checkForFullyQualifiedStep(step, params)
-		if !ok {
+		if ok {
+			log.Printf("Task %s is satisfied by environment variables and will be skipped", step.Name())
+		} else {
 			imageStepLinks = append(imageStepLinks, stepLinks...)
 		}
 		buildSteps = append(buildSteps, step)
@@ -169,9 +183,7 @@ func FromConfig(
 		buildSteps = append(buildSteps, steps.WriteParametersStep(params, paramFile))
 	}
 
-	if releaseStep != nil {
-		buildSteps = append(buildSteps, releaseStep)
-	} else {
+	if !hasReleaseStep {
 		buildSteps = append(buildSteps, release.StableImagesTagStep(imageClient, jobSpec))
 	}
 
@@ -195,14 +207,23 @@ func FromConfig(
 	return buildSteps, postSteps, nil
 }
 
+// addProvidesForStep adds any required parameters to the deferred parameters map.
+// Use this when a step may still need to run even if all parameters are provided
+// by the caller as environment variables.
+func addProvidesForStep(step api.Step, params *api.DeferredParameters) {
+	provides, link := step.Provides()
+	for name, fn := range provides {
+		params.Add(name, link, fn)
+	}
+}
+
 // checkForFullyQualifiedStep if all output parameters of this step are part of the
 // environment, replace the step with a shim that automatically provides those variables.
 // Returns true if the step was replaced.
 func checkForFullyQualifiedStep(step api.Step, params *api.DeferredParameters) (api.Step, bool) {
 	provides, link := step.Provides()
 
-	if values, ok := envHasAllParameters(provides); ok {
-		log.Printf("Task %s is satisfied by environment variables and will be skipped", step.Name())
+	if values, ok := paramsHasAllParametersAsInput(params, provides); ok {
 		step = steps.NewInputEnvironmentStep(step.Name(), values, step.Creates())
 		for k, v := range values {
 			params.Set(k, v)
@@ -388,7 +409,10 @@ func stepConfigsForBuild(config *api.ReleaseBuildConfiguration, jobSpec *api.Job
 
 	for i := range config.Tests {
 		test := &config.Tests[i]
-		if test.ContainerTestConfiguration != nil {
+		switch {
+		case test.ContainerTestConfiguration != nil:
+			buildSteps = append(buildSteps, api.StepConfiguration{TestStepConfiguration: test})
+		case test.OpenshiftInstallerClusterTestConfiguration != nil && test.OpenshiftInstallerClusterTestConfiguration.Upgrade:
 			buildSteps = append(buildSteps, api.StepConfiguration{TestStepConfiguration: test})
 		}
 	}
@@ -430,19 +454,22 @@ func createStepConfigForGitSource(target api.ProjectDirectoryImageBuildInputs, j
 	}
 }
 
-func envHasAllParameters(params map[string]func() (string, error)) (map[string]string, bool) {
+func paramsHasAllParametersAsInput(p api.Parameters, params map[string]func() (string, error)) (map[string]string, bool) {
 	if len(params) == 0 {
 		return nil, false
 	}
 	var values map[string]string
 	for k := range params {
-		v, ok := os.LookupEnv(k)
-		if !ok {
+		if !p.HasInput(k) {
 			return nil, false
 		}
 		if values == nil {
 			values = make(map[string]string)
 		}
+		v, err := p.Get(k)
+		if err != nil {
+			return nil, false
+		}
 		values[k] = v
 	}
 	return values, true