Add raw deployment deployer (#3075)

* Add kubernetes deployment deployer

* Run update-codegen.sh

* Remove unneeded function

* Address build issues

* Run make schema-generate

* Get logs by selector in integration_test

This allows to get the logs of the deployment too

* Add implementations for Lister, Describer and Remover for raw deployments

* Use correct deploytype in integration test

* tmp: Use http scheme for url in describer

* Fix receiving pod logs correctly in test

* Fix deployer integration test

* Wait for pods of deployment to be ready too

* Move Deployer, Describer, Lister and Remover int tests to correct package

* Log receiver also check existing pods

* Revert "Log receiver also check existing pods"

This reverts commit a9bf274.

* tmp: disable cleanup to get pod logs / status and add more logs

* Revert "tmp: disable cleanup to get pod logs / status and add more logs"

This reverts commit b498724.

* Give a bit more time to collect logs

* Add multi deployer, -lister, -remover

* Move deployer, lister, describer and removers in separate packages

* Use integration tests in k8s packages too

* Fix import cycle issue

* Fix integration tests

* Remove unneeded test

* Run all deployer integration tests on both deployer implementations

* Add small delay in test

* Revert "Add small delay in test"

This reverts commit 2aec3f4.

* Wait for deployments to be up

* wait for ready pods instead of checking only if containers are running

* tmp: collect pod status and logs

* Revert "tmp: collect pod status and logs"

This reverts commit 8bc2a0b.

* Add deploy-type information in description

* Poll for pod logs instead of time.sleep

* Switch to PollUntilContextTimeout in wait functions

* Make sure no pods of old replicas of the deployment are running anymore in WaitForDeploymentsAvailable functions

* Change clients WithLister|Describer|Remover methods to variadic arguments

* Move Deployers, Listers, Describers and Removers back into knative/k8s package

* [WIP] Resolve some cyclic dependencies

* Remove subscribe functionality from raw deployer to remove dependencies

* Some more cleanup and import alias fixes

* Rename deploy-type arg to deployer to align with other arg names

* Fix build issues in cmd unit tests

* Adjust integration tests to not test func subscribe by default and skip test for kubernetes deployer

* Add shell completion for deployer selection

* Undo unneeded changes in imports ordering

* Move SynchronizedBuffer to k8s package

* Readd Test_processValue unit test

* Use constants for waiting timeout

* Use onClusterFix for raw deployer too

* Remove unneeded build tags

* Use TestInt_ pattern for integration tests (and helper functions)

* Use dot import for pkg/testing

* Use dot import for pkg/testing/k8s

* Use fn alias for pkg/functions import for consistency

* Update outdated comment

* Some more TestInt_xyz replaces

* Mark test helper functions with t.Helper()

* Removed build tags in other int_test_helpers.go

* Use dot imports for some testing packages

* Move testing/k8s to testing package

* Revert "Move testing/k8s to testing package"

This reverts commit 9782f41.

* Update describe to return a ErrNotHandled error

* Update remover to return a ErrNotHandled error

* Update lister to only return resultlist and error (remove handled/bool return value)

* Move IsCRDNotFoundError() to knative package and make specific to Knative

* Add +build integration tag on integration test files

* Remove misleading comment

* Fix build issues

* Add a few more comments to the Describe, Remove and List usage about responsibility

* Remove outdated package comment

* Remove need to specify deployer in function programmatic declaration

* create Trigger in test only for Knative Service as we don't support it yet for the raw deployer anyhow

* Use correct API version for triggers subscriber ref

* Ignore golang-lint dot import errors in testing helpers

Author: creydr

cmd/client.go

@@ -58,16 +58,16 @@ func NewClient(cfg ClientConfig, options ...fn.Option) (*fn.Client, func()) {
 	var (
 		t  = newTransport(cfg.InsecureSkipVerify)    // may provide a custom impl which proxies
 		c  = newCredentialsProvider(config.Dir(), t) // for accessing registries
-		d  = newKnativeDeployer(cfg.Verbose)
+		d  = newKnativeDeployer(cfg.Verbose)         // default deployer (can be overridden via options)
 		pp = newTektonPipelinesProvider(c, cfg.Verbose)
 		o  = []fn.Option{ // standard (shared) options for all commands
 			fn.WithVerbose(cfg.Verbose),
 			fn.WithTransport(t),
 			fn.WithRepositoriesPath(config.RepositoriesPath()),
 			fn.WithBuilder(buildpacks.NewBuilder(buildpacks.WithVerbose(cfg.Verbose))),
-			fn.WithRemover(knative.NewRemover(cfg.Verbose)),
-			fn.WithDescriber(knative.NewDescriber(cfg.Verbose)),
-			fn.WithLister(knative.NewLister(cfg.Verbose)),
+			fn.WithRemovers(knative.NewRemover(cfg.Verbose), k8s.NewRemover(cfg.Verbose)),
+			fn.WithDescribers(knative.NewDescriber(cfg.Verbose), k8s.NewDescriber(cfg.Verbose)),
+			fn.WithListers(knative.NewLister(cfg.Verbose), k8s.NewLister(cfg.Verbose)),
 			fn.WithDeployer(d),
 			fn.WithPipelinesProvider(pp),
 			fn.WithPusher(docker.NewPusher(
@@ -135,6 +135,15 @@ func newKnativeDeployer(verbose bool) fn.Deployer {
 	return knative.NewDeployer(options...)
 }
 
+func newK8sDeployer(verbose bool) fn.Deployer {
+	options := []k8s.DeployerOpt{
+		k8s.WithDeployerVerbose(verbose),
+		k8s.WithDeployerDecorator(deployDecorator{}),
+	}
+
+	return k8s.NewDeployer(options...)
+}
+
 type deployDecorator struct {
 	oshDec k8s.OpenshiftMetadataDecorator
 }

cmd/client_test.go

@@ -21,10 +21,10 @@ func Test_NewTestClient(t *testing.T) {
 	)
 
 	// Factory constructor options which should be used when invoking later
-	clientFn := NewTestClient(fn.WithRemover(remover))
+	clientFn := NewTestClient(fn.WithRemovers(remover))
 
 	// Factory should ignore options provided when invoking
-	client, _ := clientFn(ClientConfig{}, fn.WithDescriber(describer))
+	client, _ := clientFn(ClientConfig{}, fn.WithDescribers(describer))
 
 	// Trigger an invocation of the mocks by running the associated client
 	// methods which depend on them

cmd/completion_util.go

@@ -9,21 +9,28 @@ import (
 	"strings"
 
 	"github.com/spf13/cobra"
-
 	fn "knative.dev/func/pkg/functions"
+	"knative.dev/func/pkg/k8s"
 	"knative.dev/func/pkg/knative"
 )
 
 func CompleteFunctionList(cmd *cobra.Command, args []string, toComplete string) (strings []string, directive cobra.ShellCompDirective) {
-	lister := knative.NewLister(false)
+	listers := []fn.Lister{
+		knative.NewLister(false),
+		k8s.NewLister(false),
+	}
 
-	list, err := lister.List(cmd.Context(), "")
-	if err != nil {
-		directive = cobra.ShellCompDirectiveError
-		return
+	items := []fn.ListItem{}
+	for _, lister := range listers {
+		list, err := lister.List(cmd.Context(), "")
+		if err != nil {
+			directive = cobra.ShellCompDirectiveError
+			return
+		}
+		items = append(items, list...)
 	}
 
-	for _, item := range list {
+	for _, item := range items {
 		strings = append(strings, item.Name)
 	}
 	directive = cobra.ShellCompDirectiveDefault
@@ -158,3 +165,26 @@ func CompleteBuilderList(cmd *cobra.Command, args []string, complete string) (ma
 
 	return
 }
+
+func CompleteDeployerList(cmd *cobra.Command, args []string, complete string) (matches []string, d cobra.ShellCompDirective) {
+	deployers := []string{
+		knative.KnativeDeployerName,
+		k8s.KubernetesDeployerName,
+	}
+
+	d = cobra.ShellCompDirectiveNoFileComp
+	matches = []string{}
+
+	if len(complete) == 0 {
+		matches = deployers
+		return
+	}
+
+	for _, b := range deployers {
+		if strings.HasPrefix(b, complete) {
+			matches = append(matches, b)
+		}
+	}
+
+	return
+}

cmd/delete_test.go

@@ -53,7 +53,7 @@ func TestDelete_Default(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	cmd := NewDeleteCmd(NewTestClient(fn.WithRemover(remover)))
+	cmd := NewDeleteCmd(NewTestClient(fn.WithRemovers(remover)))
 	cmd.SetArgs([]string{})
 	if err := cmd.Execute(); err != nil {
 		t.Fatal(err)
@@ -104,7 +104,7 @@ func TestDelete_ByName(t *testing.T) {
 
 	// Create a command with a client constructor fn that instantiates a client
 	// with a mocked remover.
-	cmd := NewDeleteCmd(NewTestClient(fn.WithRemover(remover)))
+	cmd := NewDeleteCmd(NewTestClient(fn.WithRemovers(remover)))
 	cmd.SetArgs([]string{testname}) // run: func delete <name>
 	if err := cmd.Execute(); err != nil {
 		t.Fatal(err)
@@ -132,7 +132,7 @@ func TestDelete_Namespace(t *testing.T) {
 		return nil
 	}
 
-	cmd := NewDeleteCmd(NewTestClient(fn.WithRemover(remover)))
+	cmd := NewDeleteCmd(NewTestClient(fn.WithRemovers(remover)))
 	cmd.SetArgs([]string{testname, "--namespace", namespace})
 	if err := cmd.Execute(); err != nil {
 		t.Fatal(err)
@@ -177,7 +177,7 @@ func TestDelete_NamespaceFlagPriority(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	cmd := NewDeleteCmd(NewTestClient(fn.WithRemover(remover)))
+	cmd := NewDeleteCmd(NewTestClient(fn.WithRemovers(remover)))
 	cmd.SetArgs([]string{testname, "--namespace", namespace2})
 	if err := cmd.Execute(); err != nil {
 		t.Fatal(err)
@@ -233,7 +233,7 @@ created: 2021-01-01T00:00:00+00:00
 
 	// Command with a Client constructor that returns  client with the
 	// mocked remover.
-	cmd := NewDeleteCmd(NewTestClient(fn.WithRemover(remover)))
+	cmd := NewDeleteCmd(NewTestClient(fn.WithRemovers(remover)))
 	cmd.SetArgs([]string{}) // Do not use test command args
 
 	// Execute the command simulating no arguments.
@@ -277,7 +277,7 @@ func TestDelete_ByPath(t *testing.T) {
 	}
 
 	// Command with a Client constructor using the mock remover.
-	cmd := NewDeleteCmd(NewTestClient(fn.WithRemover(remover)))
+	cmd := NewDeleteCmd(NewTestClient(fn.WithRemovers(remover)))
 
 	// Execute the command only with the path argument
 	cmd.SetArgs([]string{"-p", root})
@@ -303,7 +303,7 @@ func TestDelete_NameAndPathExclusivity(t *testing.T) {
 	remover := mock.NewRemover()
 
 	// Command with a Client constructor using the mock remover.
-	cmd := NewDeleteCmd(NewTestClient(fn.WithRemover(remover)))
+	cmd := NewDeleteCmd(NewTestClient(fn.WithRemovers(remover)))
 
 	// Capture command output for inspection
 	buf := new(bytes.Buffer)

cmd/deploy.go

@@ -15,11 +15,11 @@ import (
 	"github.com/spf13/cobra"
 	"k8s.io/apimachinery/pkg/api/resource"
 	"knative.dev/client/pkg/util"
-
 	"knative.dev/func/pkg/builders"
 	"knative.dev/func/pkg/config"
 	fn "knative.dev/func/pkg/functions"
 	"knative.dev/func/pkg/k8s"
+	"knative.dev/func/pkg/knative"
 	"knative.dev/func/pkg/utils"
 )
 
@@ -132,7 +132,7 @@ EXAMPLES
 		PreRunE: bindEnv("build", "build-timestamp", "builder", "builder-image",
 			"base-image", "confirm", "domain", "env", "git-branch", "git-dir",
 			"git-url", "image", "namespace", "path", "platform", "push", "pvc-size",
-			"service-account", "registry", "registry-insecure", "remote",
+			"service-account", "deployer", "registry", "registry-insecure", "remote",
 			"username", "password", "token", "verbose", "remote-storage-class"),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			return runDeploy(cmd, newClient)
@@ -193,6 +193,8 @@ EXAMPLES
 		"When triggering a remote deployment, set a custom volume size to allocate for the build operation ($FUNC_PVC_SIZE)")
 	cmd.Flags().String("service-account", f.Deploy.ServiceAccountName,
 		"Service account to be used in the deployed function ($FUNC_SERVICE_ACCOUNT)")
+	cmd.Flags().String("deployer", f.Deploy.Deployer,
+		fmt.Sprintf("Type of deployment to use: '%s' for Knative Service (default) or '%s' for Kubernetes Deployment ($FUNC_DEPLOY_TYPE)", knative.KnativeDeployerName, k8s.KubernetesDeployerName))
 	// Static Flags:
 	// Options which have static defaults only (not globally configurable nor
 	// persisted with the function)
@@ -234,6 +236,10 @@ EXAMPLES
 		fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err)
 	}
 
+	if err := cmd.RegisterFlagCompletionFunc("deployer", CompleteDeployerList); err != nil {
+		fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err)
+	}
+
 	return cmd
 }
 
@@ -660,6 +666,9 @@ type deployConfig struct {
 	//Service account to be used in deployed function
 	ServiceAccountName string
 
+	// Deployer specifies the type of deployment: "knative" or "raw"
+	Deployer string
+
 	// Remote indicates the deployment (and possibly build) process are to
 	// be triggered in a remote environment rather than run locally.
 	Remote bool
@@ -693,6 +702,7 @@ func newDeployConfig(cmd *cobra.Command) deployConfig {
 		PVCSize:            viper.GetString("pvc-size"),
 		Timestamp:          viper.GetBool("build-timestamp"),
 		ServiceAccountName: viper.GetString("service-account"),
+		Deployer:           viper.GetString("deployer"),
 	}
 	// NOTE: .Env should be viper.GetStringSlice, but this returns unparsed
 	// results and appears to be an open issue since 2017:
@@ -727,6 +737,7 @@ func (c deployConfig) Configure(f fn.Function) (fn.Function, error) {
 	f.Build.Git.Revision = c.GitBranch // TODO: should match; perhaps "refSpec"
 	f.Build.RemoteStorageClass = c.RemoteStorageClass
 	f.Deploy.ServiceAccountName = c.ServiceAccountName
+	f.Deploy.Deployer = c.Deployer
 	f.Local.Remote = c.Remote
 
 	// PVCSize
@@ -899,6 +910,32 @@ func (c deployConfig) Validate(cmd *cobra.Command) (err error) {
 	return
 }
 
+// clientOptions returns client options specific to deploy, including the appropriate deployer
+func (c deployConfig) clientOptions() ([]fn.Option, error) {
+	// Start with build config options
+	o, err := c.buildConfig.clientOptions()
+	if err != nil {
+		return o, err
+	}
+
+	// Add the appropriate deployer based on deploy type
+	deployer := c.Deployer
+	if deployer == "" {
+		deployer = knative.KnativeDeployerName // default to knative for backwards compatibility
+	}
+
+	switch deployer {
+	case knative.KnativeDeployerName:
+		o = append(o, fn.WithDeployer(newKnativeDeployer(c.Verbose)))
+	case k8s.KubernetesDeployerName:
+		o = append(o, fn.WithDeployer(newK8sDeployer(c.Verbose)))
+	default:
+		return o, fmt.Errorf("unsupported deploy type: %s (supported: %s, %s)", deployer, knative.KnativeDeployerName, k8s.KubernetesDeployerName)
+	}
+
+	return o, nil
+}
+
 // printDeployMessages to the output.  Non-error deployment messages.
 func printDeployMessages(out io.Writer, f fn.Function) {
 	digest, err := isDigested(f.Image)

cmd/deploy_test.go

@@ -1959,7 +1959,7 @@ func TestDeploy_NoErrorOnOldFunctionNotFound(t *testing.T) {
 	}
 	clientFn := NewTestClient(
 		fn.WithDeployer(mock.NewDeployer()),
-		fn.WithRemover(remover),
+		fn.WithRemovers(remover),
 	)
 
 	// Create a basic go Function

cmd/describe.go

@@ -179,6 +179,9 @@ func (i info) Human(w io.Writer) error {
 		fmt.Fprintf(w, "  %v\n", route)
 	}
 
+	fmt.Fprintln(w, "Deployer:")
+	fmt.Fprintf(w, "  %v\n", i.Deployer)
+
 	if len(i.Subscriptions) > 0 {
 		fmt.Fprintln(w, "Subscriptions (Source, Type, Broker):")
 		for _, s := range i.Subscriptions {
@@ -204,6 +207,8 @@ func (i info) Plain(w io.Writer) error {
 		fmt.Fprintf(w, "Route %v\n", route)
 	}
 
+	fmt.Fprintf(w, "Deployer %v\n", i.Deployer)
+
 	if len(i.Subscriptions) > 0 {
 		for _, s := range i.Subscriptions {
 			fmt.Fprintf(w, "Subscription %v %v %v\n", s.Source, s.Type, s.Broker)

cmd/describe_test.go

@@ -17,7 +17,7 @@ func TestDescribe_Default(t *testing.T) {
 	_ = FromTempDirectory(t)
 	describer := mock.NewDescriber()
 
-	cmd := NewDescribeCmd(NewTestClient(fn.WithDescriber(describer)))
+	cmd := NewDescribeCmd(NewTestClient(fn.WithDescribers(describer)))
 	cmd.SetArgs([]string{})
 	err := cmd.Execute()
 
@@ -52,7 +52,7 @@ func TestDescribe_Undeployed(t *testing.T) {
 
 	describer := mock.NewDescriber()
 
-	cmd := NewDescribeCmd(NewTestClient(fn.WithDescriber(describer)))
+	cmd := NewDescribeCmd(NewTestClient(fn.WithDescribers(describer)))
 	cmd.SetArgs([]string{})
 	if err := cmd.Execute(); err != nil {
 		t.Fatal(err)
@@ -78,7 +78,7 @@ func TestDescribe_ByName(t *testing.T) {
 		return fn.Instance{}, nil
 	}
 
-	cmd := NewDescribeCmd(NewTestClient(fn.WithDescriber(describer)))
+	cmd := NewDescribeCmd(NewTestClient(fn.WithDescribers(describer)))
 	cmd.SetArgs([]string{testname})
 	if err := cmd.Execute(); err != nil {
 		t.Fatal(err)
@@ -111,9 +111,9 @@ func TestDescribe_ByProject(t *testing.T) {
 		if name != expected {
 			t.Fatalf("expected describer to receive name %q, got %q", expected, name)
 		}
-		return
+		return fn.Instance{}, nil
 	}
-	cmd := NewDescribeCmd(NewTestClient(fn.WithDescriber(describer)))
+	cmd := NewDescribeCmd(NewTestClient(fn.WithDescribers(describer)))
 	cmd.SetArgs([]string{})
 	if err := cmd.Execute(); err != nil {
 		t.Fatal(err)
@@ -124,7 +124,7 @@ func TestDescribe_ByProject(t *testing.T) {
 // and a path will generate an error.
 func TestDescribe_NameAndPathExclusivity(t *testing.T) {
 	d := mock.NewDescriber()
-	cmd := NewDescribeCmd(NewTestClient(fn.WithDescriber(d)))
+	cmd := NewDescribeCmd(NewTestClient(fn.WithDescribers(d)))
 	cmd.SetArgs([]string{"-p", "./testpath", "testname"})
 	if err := cmd.Execute(); err == nil {
 		t.Fatalf("expected error on conflicting flags not received")

cmd/list.go

@@ -187,9 +187,9 @@ func (items listItems) Plain(w io.Writer) error {
 	tabWriter := tabwriter.NewWriter(w, 0, 8, 2, ' ', 0)
 	defer tabWriter.Flush()
 
-	fmt.Fprintf(tabWriter, "%s\t%s\t%s\t%s\t%s\n", "NAME", "NAMESPACE", "RUNTIME", "URL", "READY")
+	fmt.Fprintf(tabWriter, "%s\t%s\t%s\t%s\t%s\t%s\n", "NAME", "NAMESPACE", "RUNTIME", "DEPLOYER", "URL", "READY")
 	for _, item := range items {
-		fmt.Fprintf(tabWriter, "%s\t%s\t%s\t%s\t%s\n", item.Name, item.Namespace, item.Runtime, item.URL, item.Ready)
+		fmt.Fprintf(tabWriter, "%s\t%s\t%s\t%s\t%s\t%s\n", item.Name, item.Namespace, item.Runtime, item.Deployer, item.URL, item.Ready)
 	}
 	return nil
 }

cmd/list_test.go

@@ -71,7 +71,7 @@ func TestList_Namespace(t *testing.T) {
 
 			// Create an instance of the command which sets the flags
 			// according to the test case
-			cmd := NewListCmd(NewTestClient(fn.WithLister(lister)))
+			cmd := NewListCmd(NewTestClient(fn.WithListers(lister)))
 			args := []string{}
 			if test.namespace != "" {
 				args = append(args, "--namespace", test.namespace)