Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func Test_NewTestClient(t *testing.T) {

// Trigger an invocation of the mocks by running the associated client
// methods which depend on them
err := client.Remove(t.Context(), "myfunc", "myns", fn.Function{}, true)
_, err := client.Remove(t.Context(), "myfunc", "myns", fn.Function{}, true)
if err != nil {
t.Fatal(err)
}
Expand Down
13 changes: 10 additions & 3 deletions cmd/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ This command undeploys a function from the cluster. By default the function from
the project in the current directory is undeployed. Alternatively either the name
of the function can be given as argument or the project path provided with --path.

No local files are deleted.
No local files are deleted. When undeploying by project (not by name), on success
the local func.yaml is updated to reflect that the function is no longer deployed.
`,
Example: `
# Undeploy the function defined in the local directory
Expand Down Expand Up @@ -82,13 +83,19 @@ func runDelete(cmd *cobra.Command, args []string, newClient ClientFactory) (err
defer done()

if cfg.Name != "" { // Delete by name if provided
return client.Remove(cmd.Context(), cfg.Name, cfg.Namespace, fn.Function{}, cfg.All)
_, err = client.Remove(cmd.Context(), cfg.Name, cfg.Namespace, fn.Function{}, cfg.All)
return err
} else { // Otherwise; delete the function at path (cwd by default)
f, err := fn.NewFunction(cfg.Path)
if err != nil {
return err
}
return client.Remove(cmd.Context(), "", "", f, cfg.All)
// updates f.Deploy.<Deployer|Namespace> (clears them on success)
f, err = client.Remove(cmd.Context(), "", "", f, cfg.All)
if err != nil {
return err
}
return f.Write()
}
}

Expand Down
178 changes: 178 additions & 0 deletions cmd/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import (
"strings"
"testing"

"knative.dev/func/pkg/deployers"
fn "knative.dev/func/pkg/functions"
"knative.dev/func/pkg/keda"
"knative.dev/func/pkg/mock"
. "knative.dev/func/pkg/testing"
)
Expand Down Expand Up @@ -340,3 +342,179 @@ func TestDelete_NoFunctionAtPath(t *testing.T) {
t.Fatalf("unexpected error message: %v", err)
}
}

// TestDelete_ByProjectClearsDeployedMarker ensures that a successful
// path-based delete persists a cleared function on disk so we have the
// function locally and on-cluster synced.
func TestDelete_ByProjectClearsDeployedMarker(t *testing.T) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, this could be a test in core, because it would be doing this clearing for every deployer (even the mock on the test)

root := FromTempDirectory(t)
f := fn.Function{
Root: root,
Runtime: "go",
Registry: TestRegistry,
Deployer: keda.KedaDeployerName, // intent - how to deploy
Deploy: fn.DeploySpec{Namespace: "myns", Deployer: keda.KedaDeployerName},
}
f, err := fn.New().Init(f)
if err != nil {
t.Fatal(err)
}
if err = f.Write(); err != nil {
t.Fatal(err)
}

remover := mock.NewRemover()
remover.RemoveFn = func(_, _ string) error { return nil }

cmd := NewDeleteCmd(NewTestClient(fn.WithRemovers(remover)))
cmd.SetArgs([]string{})
if err := cmd.Execute(); err != nil {
t.Fatal(err)
}
if !remover.RemoveInvoked {
t.Fatal("fn.Remover not invoked")
}

loaded, err := fn.NewFunction(root)
if err != nil {
t.Fatal(err)
}
if loaded.Deploy.Namespace != "" {
t.Fatalf("expected Deploy.Namespace cleared after a successful undeploy, got %q", loaded.Deploy.Namespace)
}
if loaded.Deploy.Deployer != "" {
t.Fatalf("expected Deploy.Deployer cleared after a successful undeploy, got %q", loaded.Deploy.Deployer)
}
if loaded.Deployer != keda.KedaDeployerName {
t.Fatalf("expected the intended Deployer preserved as a remembered choice, got %q", loaded.Deployer)
}
}

// TestDelete_ByNameLeavesLocalFunctionUntouched ensures delete-by-name does NOT
// modify the local function's deployed state, even when a local function
// exists in the working directory. This is the distinction we make, its the
// caller's responsibility (of client.Remove) to deal with this.
func TestDelete_ByNameLeavesLocalFunctionUntouched(t *testing.T) {
root := FromTempDirectory(t)
f := fn.Function{
Root: root,
Runtime: "go",
Registry: TestRegistry,
Name: "localfn",
Deploy: fn.DeploySpec{Namespace: "myns", Deployer: keda.KedaDeployerName},
}
f, err := fn.New().Init(f)
if err != nil {
t.Fatal(err)
}
if err = f.Write(); err != nil {
t.Fatal(err)
}

remover := mock.NewRemover()
remover.RemoveFn = func(n, _ string) error {
if n != "otherfn" {
t.Fatalf("expected delete-by-name target %q, got %q", "otherfn", n)
}
return nil
}

cmd := NewDeleteCmd(NewTestClient(fn.WithRemovers(remover)))
cmd.SetArgs([]string{"otherfn"}) // explicit name, distinct from the local function
if err := cmd.Execute(); err != nil {
t.Fatal(err)
}
if !remover.RemoveInvoked {
t.Fatal("fn.Remover not invoked")
}

loaded, err := fn.NewFunction(root)
if err != nil {
t.Fatal(err)
}
if loaded.Deploy.Namespace != "myns" {
t.Fatalf("expected delete-by-name to leave the local function untouched, Deploy.Namespace changed to %q", loaded.Deploy.Namespace)
}
}

// TestDelete_ByProjectPreservesDeployerForRedeploy ensures that redeploying
// after removal keeps the INTENT deployer intact and functional.
func TestDelete_ByProjectPreservesDeployerForRedeploy(t *testing.T) {
root := FromTempDirectory(t)
if _, err := fn.New().Init(fn.Function{Runtime: "go", Root: root, Registry: TestRegistry}); err != nil {
t.Fatal(err)
}

// Deploy with keda
deployCmd := NewDeployCmd(NewTestClient(fn.WithDeployer(mock.NewDeployer())))
deployCmd.SetArgs([]string{"--deployer", keda.KedaDeployerName, "--namespace", "myns"})
if err := deployCmd.Execute(); err != nil {
t.Fatal(err)
}

// Undeploy by project
remover := mock.NewRemover()
remover.RemoveFn = func(_, _ string) error { return nil }
deleteCmd := NewDeleteCmd(NewTestClient(fn.WithRemovers(remover)))
deleteCmd.SetArgs([]string{})
if err := deleteCmd.Execute(); err != nil {
t.Fatal(err)
}

// Flag-less redeploy: must reuse the persisted keda deployer.
deployer := mock.NewDeployer()
redeployCmd := NewDeployCmd(NewTestClient(fn.WithDeployer(deployer)))
redeployCmd.SetArgs([]string{})
if err := redeployCmd.Execute(); err != nil {
t.Fatalf("expected a flag-less redeploy after delete to succeed, got: %v", err)
}
if !deployer.DeployInvoked {
t.Fatal("expected the deployer to be invoked on the redeploy")
}

loaded, err := fn.NewFunction(root)
if err != nil {
t.Fatal(err)
}
if loaded.Deploy.Deployer != keda.KedaDeployerName {
t.Fatalf("expected the flag-less redeploy to reuse the persisted %q deployer, got %q", keda.KedaDeployerName, loaded.Deploy.Deployer)
}
}

// TestDelete_ByProjectThenRedeployWithDifferentDeployerNotBlocked ensures that
// undeploy removes "deployed status" effectively unblocking the subsequent
// re-deployment with different deployer
func TestDelete_ByProjectThenRedeployWithDifferentDeployerNotBlocked(t *testing.T) {
root := FromTempDirectory(t)
f := fn.Function{
Root: root,
Runtime: "go",
Registry: TestRegistry,
Deploy: fn.DeploySpec{Namespace: "myns", Deployer: keda.KedaDeployerName},
}
f, err := fn.New().Init(f)
if err != nil {
t.Fatal(err)
}
if err := f.Write(); err != nil {
t.Fatal(err)
}

remover := mock.NewRemover()
remover.RemoveFn = func(_, _ string) error { return nil }
deleteCmd := NewDeleteCmd(NewTestClient(fn.WithRemovers(remover)))
deleteCmd.SetArgs([]string{})
if err := deleteCmd.Execute(); err != nil {
t.Fatal(err)
}

deployer := mock.NewDeployer()
deployCmd := NewDeployCmd(NewTestClient(fn.WithDeployer(deployer)))
deployCmd.SetArgs([]string{"--deployer", deployers.Kubernetes})
if err := deployCmd.Execute(); err != nil {
t.Fatalf("expected the redeploy with a different deployer to succeed after undeploy, got: %v", err)
}
if !deployer.DeployInvoked {
t.Fatal("expected the deployer to be invoked - the guard must not fire on an undeployed function")
}
}
24 changes: 13 additions & 11 deletions cmd/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"knative.dev/func/cmd/common"
"knative.dev/func/pkg/builders"
"knative.dev/func/pkg/config"
"knative.dev/func/pkg/deployers"
fn "knative.dev/func/pkg/functions"
"knative.dev/func/pkg/k8s"
"knative.dev/func/pkg/keda"
Expand Down Expand Up @@ -159,6 +160,8 @@ EXAMPLES
// contextually relevant function; but sets are flattened via cfg.Apply(f)
cmd.Flags().StringP("builder", "b", cfg.Builder,
fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s.", KnownBuilders()))
cmd.Flags().String("deployer", cfg.Deployer,
fmt.Sprintf("Type of deployment to use: '%s' for Knative Service, '%s' for Kubernetes Deployment, or '%s' for Deployment with a KEDA HTTP scaler ($FUNC_DEPLOYER)", deployers.Knative, deployers.Kubernetes, deployers.Keda))
cmd.Flags().StringP("registry", "r", cfg.Registry,
"Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY)")
cmd.Flags().Bool("registry-insecure", cfg.RegistryInsecure, "Skip TLS certificate verification when communicating in HTTPS with the registry. The value is persisted over consecutive runs ($FUNC_REGISTRY_INSECURE)")
Expand Down Expand Up @@ -197,8 +200,6 @@ EXAMPLES
"Service account to be used in the deployed function ($FUNC_SERVICE_ACCOUNT)")
cmd.Flags().String("image-pull-secret", f.Deploy.ImagePullSecret,
"Image pull secret to use when the function's image is in a private registry ($FUNC_IMAGE_PULL_SECRET)")
cmd.Flags().String("deployer", f.Deploy.Deployer,
fmt.Sprintf("Type of deployment to use: '%s' for Knative Service (default), '%s' for Kubernetes Deployment or '%s' for Deployment with a Keda HTTP scaler ($FUNC_DEPLOY_TYPE)", knative.KnativeDeployerName, k8s.KubernetesDeployerName, keda.KedaDeployerName))
// Static Flags:
// Options which have static defaults only (not globally configurable nor
// persisted with the function)
Expand Down Expand Up @@ -284,6 +285,12 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) {
// Warn if registry changed but registryInsecure is still true
warnRegistryInsecureChange(cmd.OutOrStderr(), cfg.Registry, f)

// Back-compat: a function deployed before the deployer was recorded has a
// namespace but no deployer, which historically could only mean knative.
if f.Deploy.Namespace != "" && f.Deploy.Deployer == "" {
f.Deploy.Deployer = deployers.Knative
}

if f, err = cfg.Configure(f); err != nil { // Updates f with deploy cfg
return
}
Expand Down Expand Up @@ -617,7 +624,7 @@ func (c deployConfig) Configure(f fn.Function) (fn.Function, error) {
f.Build.RemoteStorageClass = c.RemoteStorageClass
f.Deploy.ServiceAccountName = c.ServiceAccountName
f.Deploy.ImagePullSecret = c.ImagePullSecret
f.Deploy.Deployer = c.Deployer
f.Deployer = c.Deployer
f.Deploy.ManagementDisabled = c.ManagementDisabled
f.Local.Remote = c.Remote

Expand Down Expand Up @@ -804,21 +811,16 @@ func (c deployConfig) clientOptions() ([]fn.Option, error) {
// This is needed for remote builds (deploy --remote)
o = append(o, fn.WithPipelinesProvider(newTektonPipelinesProvider(creds, c.Verbose, t)))

// Add the appropriate deployer based on deploy type
deployer := c.Deployer
if deployer == "" {
deployer = knative.KnativeDeployerName // default to knative for backwards compatibility
}

switch deployer {
// Add the appropriate deployer based on deploy type.
switch c.Deployer {
case knative.KnativeDeployerName:
o = append(o, fn.WithDeployer(newKnativeDeployer(c.Verbose)))
case k8s.KubernetesDeployerName:
o = append(o, fn.WithDeployer(newK8sDeployer(c.Verbose)))
case keda.KedaDeployerName:
o = append(o, fn.WithDeployer(newKedaDeployer(c.Verbose)))
default:
return o, fmt.Errorf("unsupported deploy type: %s (supported: %s, %s, %s)", deployer, knative.KnativeDeployerName, k8s.KubernetesDeployerName, keda.KedaDeployerName)
return o, fmt.Errorf("unsupported deploy type: %s (supported: %s, %s, %s)", c.Deployer, knative.KnativeDeployerName, k8s.KubernetesDeployerName, keda.KedaDeployerName)
}

return o, nil
Expand Down
Loading
Loading