Propagate Remote With Context

We already do this with Kubernetes clients, where they are attached to
the context and propagated to descendant provisioners so the scope
magically updates as we traverse the call graph.  Do the same for the
remote, which a) makes things cleaner and we don't need to manually
propagate to child metadata, and b) gives you access to a whole host
more information e.g. the Kubernetes endpoint for troublesome
provisioners such as Cilium.  While we are here, remove the simpliar
abomination that this background deletion.

charts/core/Chart.yaml

@@ -4,8 +4,8 @@ description: A Helm chart for deploying Unikorn Core
 
 type: application
 
-version: v0.1.63
-appVersion: v0.1.63
+version: v0.1.64
+appVersion: v0.1.64
 
 icon: https://assets.unikorn-cloud.org/images/logos/dark-on-light/icon.svg
 

pkg/provisioners/application/provisioner.go

@@ -26,6 +26,7 @@ import (
 	clientlib "github.com/unikorn-cloud/core/pkg/client"
 	"github.com/unikorn-cloud/core/pkg/constants"
 	"github.com/unikorn-cloud/core/pkg/provisioners"
+	"github.com/unikorn-cloud/core/pkg/provisioners/remotecluster"
 	"github.com/unikorn-cloud/core/pkg/util"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -204,9 +205,9 @@ func (p *Provisioner) getValues(ctx context.Context) (interface{}, error) {
 }
 
 // getClusterID returns the destination cluster name.
-func (p *Provisioner) getClusterID() *cd.ResourceIdentifier {
-	if p.Remote != nil {
-		return p.Remote.ID()
+func (p *Provisioner) getClusterID(ctx context.Context) *cd.ResourceIdentifier {
+	if remote := remotecluster.FromContext(ctx); remote != nil {
+		return remote.ID()
 	}
 
 	return nil
@@ -230,7 +231,7 @@ func (p *Provisioner) generateApplication(ctx context.Context) (*cd.HelmApplicat
 		Release:       p.getReleaseName(ctx),
 		Parameters:    parameters,
 		Values:        values,
-		Cluster:       p.getClusterID(),
+		Cluster:       p.getClusterID(ctx),
 		Namespace:     p.namespace,
 		AllowDegraded: p.allowDegraded,
 	}
@@ -306,7 +307,7 @@ func (p *Provisioner) Provision(ctx context.Context) error {
 		return err
 	}
 
-	log.Info("provisioning application", "application", p.Name, "remote", p.Remote)
+	log.Info("provisioning application", "application", p.Name)
 
 	// Convert the generic object type into what's expected by the driver interface.
 	id, err := p.getResourceID(ctx)
@@ -351,7 +352,7 @@ func (p *Provisioner) Deprovision(ctx context.Context) error {
 		return err
 	}
 
-	if err := cd.FromContext(ctx).DeleteHelmApplication(ctx, id, p.BackgroundDelete); err != nil {
+	if err := cd.FromContext(ctx).DeleteHelmApplication(ctx, id, remotecluster.BackgroundDeletionFromContext(ctx)); err != nil {
 		return err
 	}
 

pkg/provisioners/application/provisioner_test.go

@@ -33,6 +33,7 @@ import (
 	"github.com/unikorn-cloud/core/pkg/provisioners"
 	"github.com/unikorn-cloud/core/pkg/provisioners/application"
 	mockprovisioners "github.com/unikorn-cloud/core/pkg/provisioners/mock"
+	"github.com/unikorn-cloud/core/pkg/provisioners/remotecluster"
 	"github.com/unikorn-cloud/core/pkg/util"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -256,11 +257,11 @@ func TestApplicationCreateHelmExtended(t *testing.T) {
 	ctx = coreclient.NewContextWithStaticClient(ctx, tc.client)
 	ctx = cd.NewContext(ctx, driver)
 	ctx = application.NewContext(ctx, owner)
+	ctx = remotecluster.NewContext(ctx, r)
 
 	driver.EXPECT().CreateOrUpdateHelmApplication(ctx, driverAppID, driverApp).Return(provisioners.ErrYield)
 
 	provisioner := application.New(getApplicationReference).AllowDegraded()
-	provisioner.OnRemote(r)
 
 	assert.ErrorIs(t, provisioner.Provision(ctx), provisioners.ErrYield)
 }

pkg/provisioners/concurrent/provisioner.go

@@ -51,15 +51,13 @@ var _ provisioners.Provisioner = &Provisioner{}
 func (p *Provisioner) Provision(ctx context.Context) error {
 	log := log.FromContext(ctx)
 
-	log.Info("provisioning concurrency group", "group", p.Name, "remote", p.Remote)
+	log.Info("provisioning concurrency group", "group", p.Name)
 
 	group := &errgroup.Group{}
 
 	for i := range p.provisioners {
 		provisioner := p.provisioners[i]
 
-		p.PropagateOptions(provisioner)
-
 		callback := func() error {
 			// As errgroup only saves the first error, we may lose some
 			// logging information, so do this here when waiting on child
@@ -98,8 +96,6 @@ func (p *Provisioner) Deprovision(ctx context.Context) error {
 	for i := range p.provisioners {
 		provisioner := p.provisioners[i]
 
-		p.PropagateOptions(provisioner)
-
 		callback := func() error {
 			// As errgroup only saves the first error, we may lose some
 			// logging information, so do this here when waiting on child

pkg/provisioners/concurrent/provisioner_test.go

@@ -75,18 +75,13 @@ func TestConcurrentProvisionPropagate(t *testing.T) {
 
 	ctx := context.Background()
 
-	r := mock.NewMockRemoteCluster(c)
-
 	p1 := mock.NewMockProvisioner(c)
-	p1.EXPECT().OnRemote(r)
 	p1.EXPECT().Provision(ctx).Return(nil)
 
 	p2 := mock.NewMockProvisioner(c)
-	p2.EXPECT().OnRemote(r)
 	p2.EXPECT().Provision(ctx).Return(nil)
 
 	provisioner := concurrent.New("test", p1, p2)
-	provisioner.OnRemote(r)
 
 	assert.NoError(t, provisioner.Provision(ctx))
 }

pkg/provisioners/conditional/provisioner.go

@@ -52,8 +52,6 @@ var _ provisioners.Provisioner = &Provisioner{}
 func (p *Provisioner) Provision(ctx context.Context) error {
 	log := log.FromContext(ctx)
 
-	p.PropagateOptions(p.provisioner)
-
 	if !p.condition() {
 		log.Info("conditional deprovision", "provisioner", p.Name)
 
@@ -65,7 +63,5 @@ func (p *Provisioner) Provision(ctx context.Context) error {
 
 // Deprovision implements the Provision interface.
 func (p *Provisioner) Deprovision(ctx context.Context) error {
-	p.PropagateOptions(p.provisioner)
-
 	return p.provisioner.Deprovision(ctx)
 }

pkg/provisioners/interfaces.go

@@ -45,16 +45,6 @@ type Provisioner interface {
 	// ProvisionerName returns the provisioner name.
 	ProvisionerName() string
 
-	// OnRemote defines this provisioner should be run on the
-	// provided remote cluster.  All composite provisioners must
-	// propagate this.
-	OnRemote(remote RemoteCluster)
-
-	// BackgroundDeletion means we don't care about whether it's deprovisioned
-	// successfully or not, especially useful for apps living in a
-	// remote cluster that going to get terminated anyway.
-	BackgroundDeletion()
-
 	// Provision deploys the requested package.
 	// Implementations should ensure this receiver is idempotent.
 	Provision(ctx context.Context) error

pkg/provisioners/remotecluster/context.go

@@ -0,0 +1,63 @@
+/*
+Copyright 2024 the Unikorn 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 remotecluster
+
+import (
+	"context"
+
+	"github.com/unikorn-cloud/core/pkg/provisioners"
+)
+
+type key int
+
+const (
+	// remoteClusterKey sets the remote cluster so it's propagated to all
+	// descendant provisioners in the call graph.
+	remoteClusterKey key = iota
+
+	// backgroundDeletionKey is used to propagate background deletion to
+	// all descendant provisioners in the call graph.
+	backgroundDeletionKey
+)
+
+func NewContext(ctx context.Context, remote provisioners.RemoteCluster) context.Context {
+	return context.WithValue(ctx, remoteClusterKey, remote)
+}
+
+func FromContext(ctx context.Context) provisioners.RemoteCluster {
+	if value := ctx.Value(remoteClusterKey); value != nil {
+		if remote, ok := value.(provisioners.RemoteCluster); ok {
+			return remote
+		}
+	}
+
+	return nil
+}
+
+func NewContextWithBackgroundDeletion(ctx context.Context, backgroundDeletion bool) context.Context {
+	return context.WithValue(ctx, backgroundDeletionKey, backgroundDeletion)
+}
+
+func BackgroundDeletionFromContext(ctx context.Context) bool {
+	if value := ctx.Value(backgroundDeletionKey); value != nil {
+		if backgroundDeletion, ok := value.(bool); ok {
+			return backgroundDeletion
+		}
+	}
+
+	return false
+}

pkg/provisioners/remotecluster/provisioner.go

@@ -72,6 +72,14 @@ type remoteClusterProvisioner struct {
 
 	// child is the provisioner to run on the remote cluster.
 	child provisioners.Provisioner
+
+	// backgroundDeletion, if set, is propagated to descendant provisioners via
+	// the context.  At present, this is only available on remote provisioners,
+	// and is intended to be used for quickly discarding applications on dynamically
+	// provisioned clusters that will be destroyed anyway.  The one caveat is that
+	// it cannot be used with remotes where applications need to be given a chance to
+	// clean up resources that will be orphaned.
+	backgroundDeletion bool
 }
 
 // Ensure the Provisioner interface is implemented.
@@ -81,9 +89,7 @@ var _ provisioners.Provisioner = &remoteClusterProvisioner{}
 type ProvisionerOption func(p *remoteClusterProvisioner)
 
 func BackgroundDeletion(p *remoteClusterProvisioner) {
-	// TODO: This mutates the child and causes side effects, could we
-	// propagate this information via the context?
-	p.child.BackgroundDeletion()
+	p.backgroundDeletion = true
 }
 
 // GetClient gets a client from the remote generator.
@@ -169,10 +175,7 @@ func (p *remoteClusterProvisioner) Provision(ctx context.Context) error {
 	}
 
 	ctx = clientlib.NewContextWithDynamicClient(ctx, client)
-
-	// TODO: This mutates the child and causes side effects, could we
-	// Remove the applications.
-	p.child.OnRemote(p.remote.generator)
+	ctx = NewContext(ctx, p.remote.generator)
 
 	// Remote is registered, create the remote applications.
 	if err := p.child.Provision(ctx); err != nil {
@@ -202,10 +205,11 @@ func (p *remoteClusterProvisioner) Deprovision(ctx context.Context) error {
 
 	if !deprovisioned {
 		ctx = clientlib.NewContextWithDynamicClient(ctx, client)
+		ctx = NewContext(ctx, p.remote.generator)
 
-		// TODO: This mutates the child and causes side effects, could we
-		// Remove the applications.
-		p.child.OnRemote(p.remote.generator)
+		if p.backgroundDeletion {
+			ctx = NewContextWithBackgroundDeletion(ctx, true)
+		}
 
 		if err := p.child.Deprovision(ctx); err != nil {
 			return err

pkg/provisioners/serial/provisioner.go

@@ -48,11 +48,9 @@ var _ provisioners.Provisioner = &Provisioner{}
 func (p *Provisioner) Provision(ctx context.Context) error {
 	log := log.FromContext(ctx)
 
-	log.Info("provisioning serial group", "group", p.Name, "remote", p.Remote)
+	log.Info("provisioning serial group", "group", p.Name)
 
 	for _, provisioner := range p.provisioners {
-		p.PropagateOptions(provisioner)
-
 		if err := provisioner.Provision(ctx); err != nil {
 			log.Info("serial group member exited with error", "error", err, "group", p.Name, "provisioner", provisioner.ProvisionerName())
 
@@ -77,8 +75,6 @@ func (p *Provisioner) Deprovision(ctx context.Context) error {
 	for i := range p.provisioners {
 		provisioner := p.provisioners[len(p.provisioners)-(i+1)]
 
-		p.PropagateOptions(provisioner)
-
 		if err := provisioner.Deprovision(ctx); err != nil {
 			log.Info("serial group member exited with error", "error", err, "group", p.Name, "provisioner", provisioner.ProvisionerName())