Fix various controller-manager panics

cmd/okra/okra.go

@@ -416,6 +416,7 @@ func Run() error {
 	cmd.AddCommand(syncPauseCommand())
 	cmd.AddCommand(cancelPauseCommand())
 	cmd.AddCommand(updateAnalysisRunCommand())
+	cmd.AddCommand(createOrUpdateCellCommand())
 
 	var runAnalysisInput func() *analysis.RunInput
 	runAnalysis := &cobra.Command{

cmd/okra/sync_cell.go

@@ -1,17 +1,9 @@
 package okra
 
 import (
-	"fmt"
-	"strconv"
-	"strings"
-	"time"
-
-	rolloutsv1alpha1 "github.com/mumoshu/okra/api/rollouts/v1alpha1"
-	okrav1alpha1 "github.com/mumoshu/okra/api/v1alpha1"
 	"github.com/mumoshu/okra/pkg/cell"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
-	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
 func syncCellCommand() *cobra.Command {
@@ -28,118 +20,10 @@ func syncCellCommand() *cobra.Command {
 }
 
 func initSyncCellFlags(flag *pflag.FlagSet, c *cell.SyncInput) func() *cell.SyncInput {
-	var (
-		replicas            int
-		listenerARN         string
-		targetGroupSelector okrav1alpha1.TargetGroupSelector
-		canarySteps         []string
-		matchLabels         []string
-	)
-
 	flag.StringVar(&c.NS, "namespace", "", "Namespace of the target cell")
 	flag.StringVar(&c.Name, "name", "", "Name of the target cell")
-	flag.StringVar(&listenerARN, "listener-arn", "", "ARN of the target AWS Application Load Balancer Listener that is used to receive all the traffic across cluster versions")
-	flag.StringSliceVar(&matchLabels, "match-label", []string{}, "KVs of labels that is used as target group selector")
-	flag.StringSliceVar(&targetGroupSelector.VersionLabels, "version-label", []string{okrav1alpha1.DefaultVersionLabelKey}, "Key of the label that is used to indicate the version number of the target group")
-	flag.IntVar(&replicas, "", 0, "")
-	flag.StringSliceVar(&canarySteps, "canary-steps", []string{}, "List of canary step definitions. Each step is delimited by a comma(,) and can be one of \"weight=INT\", \"pause=DURATION\", and \"analysis=TEMPLATE:arg1=val1:arg2=val2\"")
 
 	return func() *cell.SyncInput {
-		spec := c.Spec.DeepCopy()
-
-		if replicas != 0 {
-			r32 := int32(replicas)
-			spec.Replicas = &r32
-		}
-
-		targetGroupSelector.MatchLabels = make(map[string]string)
-		for _, l := range matchLabels {
-			kv := strings.Split(l, "=")
-			targetGroupSelector.MatchLabels[kv[0]] = kv[1]
-		}
-
-		var cs []rolloutsv1alpha1.CanaryStep
-		for _, s := range canarySteps {
-			var kind, arg string
-
-			{
-				splits := strings.SplitN(s, "=", 2)
-
-				if len(splits) != 2 {
-					panic(fmt.Errorf("pause: unexpected number of args. got %V, wanted only one arg", splits[1:]))
-				}
-
-				kind = splits[0]
-				arg = splits[1]
-			}
-
-			var step rolloutsv1alpha1.CanaryStep
-
-			switch kind {
-			case "weight":
-				w, err := strconv.Atoi(arg)
-				if err != nil {
-					panic(fmt.Errorf("parsing weight from %s: %w", arg, err))
-				}
-
-				w32 := int32(w)
-				step.SetWeight = &w32
-			case "pause":
-				d, err := time.ParseDuration(arg)
-				if err != nil {
-					panic(fmt.Errorf("parsing duration from %s: %w", arg, err))
-				}
-
-				step.Pause = &rolloutsv1alpha1.RolloutPause{
-					Duration: &intstr.IntOrString{
-						Type:   intstr.String,
-						StrVal: d.String(),
-					},
-				}
-			case "analysis":
-				tplAndArgs := strings.Split(arg, ":")
-				tpl := tplAndArgs[0]
-
-				var args []rolloutsv1alpha1.AnalysisRunArgument
-				for _, a := range tplAndArgs[1:] {
-					kv := strings.Split(a, "=")
-
-					args = append(args, rolloutsv1alpha1.AnalysisRunArgument{
-						Name:  kv[0],
-						Value: kv[1],
-					})
-				}
-
-				step.Analysis = &rolloutsv1alpha1.RolloutAnalysis{
-					Templates: []rolloutsv1alpha1.RolloutAnalysisTemplate{
-						{
-							TemplateName: tpl,
-						},
-					},
-					Args: args,
-				}
-			default:
-				panic(fmt.Errorf("unsupported canary step kind: %s", kind))
-			}
-
-			cs = append(cs, step)
-		}
-
-		spec.UpdateStrategy = okrav1alpha1.CellUpdateStrategy{
-			Type: okrav1alpha1.CellUpdateStrategyTypeCanary,
-			Canary: &okrav1alpha1.CellUpdateStrategyCanary{
-				Steps: cs,
-			},
-		}
-
-		spec.Ingress.AWSApplicationLoadBalancer = &okrav1alpha1.CellIngressAWSApplicationLoadBalancer{
-			ListenerARN:         listenerARN,
-			TargetGroupSelector: targetGroupSelector,
-		}
-
-		input := c
-		input.Spec = *spec
-
-		return input
+		return c
 	}
 }

pkg/awsapplicationloadbalancer/alb_apply.go

@@ -11,13 +11,17 @@ import (
 	"github.com/aws/aws-sdk-go/service/elbv2"
 	"github.com/google/go-cmp/cmp"
 	"github.com/mumoshu/okra/api/v1alpha1"
+	"github.com/mumoshu/okra/pkg/awsclicompat"
 	"golang.org/x/xerrors"
 )
 
 func Sync(d SyncInput) error {
 	log.SetFlags(log.Lshortfile)
 
 	sess := d.Session
+	if sess == nil {
+		sess = awsclicompat.NewSession("", "")
+	}
 
 	sess.Config.Endpoint = &d.Address
 
@@ -36,10 +40,12 @@ func Sync(d SyncInput) error {
 		return xerrors.Errorf("calling elbv2.DescribeRules: %w", err)
 	}
 
-	priority := d.Spec.Listener.Rule.Priority
+	priority := lr.Priority
 	if priority == 0 {
 		priority = DefaultPriority
 	}
+	lr.Priority = priority
+
 	priorityStr := strconv.Itoa(priority)
 
 	var rule *elbv2.Rule

pkg/cell/cell.go

@@ -41,7 +41,7 @@ type SyncInput struct {
 	NS   string
 	Name string
 
-	Spec okrav1alpha1.CellSpec
+	Cell *okrav1alpha1.Cell
 
 	Scheme *runtime.Scheme
 	Client client.Client
@@ -55,34 +55,38 @@ func Sync(config SyncInput) error {
 		return err
 	}
 
-	albListenerARN := config.Spec.Ingress.AWSApplicationLoadBalancer.ListenerARN
-	tgSelectorMatchLabels := config.Spec.Ingress.AWSApplicationLoadBalancer.TargetGroupSelector
+	var cell okrav1alpha1.Cell
+
+	if config.Cell != nil {
+		cell = *config.Cell
+	} else {
+		if err := managementClient.Get(ctx, types.NamespacedName{Namespace: config.NS, Name: config.Name}, &cell); err != nil {
+			return err
+		}
+	}
+
+	albListenerARN := cell.Spec.Ingress.AWSApplicationLoadBalancer.ListenerARN
+	tgSelectorMatchLabels := cell.Spec.Ingress.AWSApplicationLoadBalancer.TargetGroupSelector
 	tgSelector := labels.SelectorFromSet(tgSelectorMatchLabels.MatchLabels)
 
 	var albConfig okrav1alpha1.AWSApplicationLoadBalancerConfig
 	var albConfigExists bool
 
-	var cell okrav1alpha1.Cell
-
-	if err := managementClient.Get(ctx, types.NamespacedName{Namespace: config.NS, Name: config.Name}, &cell); err != nil {
-		return err
-	}
-
-	if err := managementClient.Get(ctx, types.NamespacedName{Namespace: config.NS, Name: config.Name}, &albConfig); err != nil {
+	if err := managementClient.Get(ctx, types.NamespacedName{Namespace: cell.Namespace, Name: config.Name}, &albConfig); err != nil {
 		log.Printf("%v\n", err)
 		if !kerrors.IsNotFound(err) {
 			return err
 		}
 
-		albConfig.Namespace = config.NS
-		albConfig.Name = config.Name
+		albConfig.Namespace = cell.Namespace
+		albConfig.Name = cell.Name
 		albConfig.Spec.ListenerARN = albListenerARN
 		ctrl.SetControllerReference(&cell, &albConfig, scheme)
 	} else {
 		albConfigExists = true
 	}
 
-	labelKeys := config.Spec.Ingress.AWSApplicationLoadBalancer.TargetGroupSelector.VersionLabels
+	labelKeys := cell.Spec.Ingress.AWSApplicationLoadBalancer.TargetGroupSelector.VersionLabels
 	if len(labelKeys) == 0 {
 		labelKeys = []string{okrav1alpha1.DefaultVersionLabelKey}
 	}
@@ -128,8 +132,8 @@ func Sync(config SyncInput) error {
 
 	// Ensure there enough cluster replicas to start a canary release
 	threshold := 1
-	if config.Spec.Replicas != nil {
-		threshold = int(*config.Spec.Replicas)
+	if cell.Spec.Replicas != nil {
+		threshold = int(*cell.Spec.Replicas)
 	}
 
 	log.Printf("cell=%s/%s, albConfigExists=%v, tgSelector=%s, len(latestTGs)=%d, len(desiredTGs)=%d\n", config.NS, config.Name, albConfigExists, tgSelector.String(), len(latestTGs), len(desiredTGs))
@@ -245,7 +249,7 @@ func Sync(config SyncInput) error {
 		desiredStableTGsWeight := 100
 
 		{
-			canarySteps := config.Spec.UpdateStrategy.Canary.Steps
+			canarySteps := cell.Spec.UpdateStrategy.Canary.Steps
 
 			passedAllCanarySteps = currentCanaryTGsWeight == 100
 

pkg/controllers/cell.go

@@ -96,7 +96,7 @@ func (r *CellReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	}
 
 	config := cell.SyncInput{
-		Spec:   cellResource.Spec,
+		Cell:   &cellResource,
 		Client: r.Client,
 		Scheme: r.Scheme,
 	}