[Bugfix] Fix License Timeout (#891)

ajanikow · web-flow · commit df426fa2d983 · 2022-01-15T23:55:08.000+01:00
diff --git a/pkg/deployment/context_impl.go b/pkg/deployment/context_impl.go
@@ -253,7 +253,7 @@ func (d *Deployment) getConnConfig() (http.ConnectionConfig, error) {
 	transport := &nhttp.Transport{
 		Proxy: nhttp.ProxyFromEnvironment,
 		DialContext: (&net.Dialer{
-			Timeout:   30 * time.Second,
+			Timeout:   10 * time.Second,
 			KeepAlive: 100 * time.Millisecond,
 			DualStack: true,
 		}).DialContext,
diff --git a/pkg/deployment/deployment.go b/pkg/deployment/deployment.go
@@ -407,7 +407,7 @@ func (d *Deployment) handleArangoDeploymentUpdatedEvent(ctx context.Context) err
 	if err := newAPIObject.Spec.Validate(); err != nil {
 		d.CreateEvent(k8sutil.NewErrorEvent("Validation failed", err, d.apiObject))
 		// Try to reset object
-		if err := d.updateCRSpec(ctx, d.apiObject.Spec, true); err != nil {
+		if err := d.updateCRSpec(ctx, d.apiObject.Spec); err != nil {
 			log.Error().Err(err).Msg("Restore original spec failed")
 			d.CreateEvent(k8sutil.NewErrorEvent("Restore original failed", err, d.apiObject))
 		}
@@ -421,7 +421,7 @@ func (d *Deployment) handleArangoDeploymentUpdatedEvent(ctx context.Context) err
 	}
 
 	// Save updated spec
-	if err := d.updateCRSpec(ctx, newAPIObject.Spec, true); err != nil {
+	if err := d.updateCRSpec(ctx, newAPIObject.Spec); err != nil {
 		return errors.WithStack(errors.Newf("failed to update ArangoDeployment spec: %v", err))
 	}
 	// Save updated accepted spec
diff --git a/pkg/deployment/deployment_inspector.go b/pkg/deployment/deployment_inspector.go
@@ -163,7 +163,7 @@ func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterva
 	} else {
 		condition, exists := status.Conditions.Get(api.ConditionTypeUpToDate)
 		if checksum != status.AppliedVersion && (!exists || condition.IsTrue()) {
-			if err = d.updateCondition(ctx, api.ConditionTypeUpToDate, false, "Spec Changed", "Spec Object changed. Waiting until plan will be applied"); err != nil {
+			if err = d.updateConditionWithHash(ctx, api.ConditionTypeUpToDate, false, "Spec Changed", "Spec Object changed. Waiting until plan will be applied", checksum); err != nil {
 				return minInspectionInterval, errors.Wrapf(err, "Unable to update UpToDate condition")
 			}
 
@@ -265,9 +265,9 @@ func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterva
 			return minInspectionInterval, errors.Wrapf(err, "Unable clean plan")
 		}
 	} else if err, updated := d.reconciler.CreatePlan(ctx, cachedStatus); err != nil {
-		d.deps.Log.Info().Msgf("Plan generated, reconciling")
 		return minInspectionInterval, errors.Wrapf(err, "Plan creation failed")
 	} else if updated {
+		d.deps.Log.Info().Msgf("Plan generated, reconciling")
 		return minInspectionInterval, nil
 	}
 
@@ -284,15 +284,15 @@ func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterva
 		isUpToDate, reason := d.isUpToDateStatus()
 
 		if !isUpToDate && status.Conditions.IsTrue(api.ConditionTypeUpToDate) {
-			if err = d.updateCondition(ctx, api.ConditionTypeUpToDate, false, reason, "There are pending operations in plan or members are in restart process"); err != nil {
+			if err = d.updateConditionWithHash(ctx, api.ConditionTypeUpToDate, false, reason, "There are pending operations in plan or members are in restart process", checksum); err != nil {
 				return minInspectionInterval, errors.Wrapf(err, "Unable to update UpToDate condition")
 			}
 
 			return minInspectionInterval, nil
 		}
 
 		if isUpToDate && !status.Conditions.IsTrue(api.ConditionTypeUpToDate) {
-			if err = d.updateCondition(ctx, api.ConditionTypeUpToDate, true, "Spec is Up To Date", "Spec is Up To Date"); err != nil {
+			if err = d.updateConditionWithHash(ctx, api.ConditionTypeUpToDate, true, "Spec is Up To Date", "Spec is Up To Date", checksum); err != nil {
 				return minInspectionInterval, errors.Wrapf(err, "Unable to update UpToDate condition")
 			}
 
@@ -420,10 +420,10 @@ func (d *Deployment) triggerCRDInspection() {
 	d.inspectCRDTrigger.Trigger()
 }
 
-func (d *Deployment) updateCondition(ctx context.Context, conditionType api.ConditionType, status bool, reason, message string) error {
-	d.deps.Log.Info().Str("condition", string(conditionType)).Bool("status", status).Str("reason", reason).Str("message", message).Msg("Updated condition")
+func (d *Deployment) updateConditionWithHash(ctx context.Context, conditionType api.ConditionType, status bool, reason, message, hash string) error {
+	d.deps.Log.Info().Str("condition", string(conditionType)).Bool("status", status).Str("reason", reason).Str("message", message).Str("hash", hash).Msg("Updated condition")
 	if err := d.WithStatusUpdate(ctx, func(s *api.DeploymentStatus) bool {
-		return s.Conditions.Update(conditionType, status, reason, message)
+		return s.Conditions.UpdateWithHash(conditionType, status, reason, message, hash)
 	}); err != nil {
 		return errors.Wrapf(err, "Unable to update condition")
 	}
diff --git a/pkg/deployment/reconcile/action_set_license.go b/pkg/deployment/reconcile/action_set_license.go
@@ -51,6 +51,9 @@ type licenseSetAction struct {
 }
 
 func (a *licenseSetAction) Start(ctx context.Context) (bool, error) {
+	ctxChild, cancel := globals.GetGlobals().Timeouts().ArangoD().WithTimeout(ctx)
+	defer cancel()
+
 	log := a.log
 
 	spec := a.actionCtx.GetSpec()
@@ -76,9 +79,6 @@ func (a *licenseSetAction) Start(ctx context.Context) (bool, error) {
 		return true, nil
 	}
 
-	ctxChild, cancel := globals.GetGlobals().Timeouts().ArangoD().WithTimeout(ctx)
-	defer cancel()
-
 	c, err := a.actionCtx.GetServerClient(ctxChild, group, m.ID)
 	if !ok {
 		log.Error().Err(err).Msg("Unable to get client")
@@ -87,15 +87,15 @@ func (a *licenseSetAction) Start(ctx context.Context) (bool, error) {
 
 	client := client.NewClient(c.Connection())
 
-	if ok, err := licenseV2Compare(ctx, client, l.V2); err != nil {
+	if ok, err := licenseV2Compare(ctxChild, client, l.V2); err != nil {
 		log.Error().Err(err).Msg("Unable to verify license")
 		return true, nil
 	} else if ok {
 		// Already latest license
 		return true, nil
 	}
 
-	if err := client.SetLicense(ctx, string(l.V2), true); err != nil {
+	if err := client.SetLicense(ctxChild, string(l.V2), true); err != nil {
 		log.Error().Err(err).Msg("Unable to set license")
 		return true, nil
 	}
diff --git a/pkg/deployment/reconcile/plan_builder_generator.go b/pkg/deployment/reconcile/plan_builder_generator.go
@@ -65,6 +65,7 @@ func (d *Reconciler) generatePlanFunc(gen planGeneratorFunc, planner planner) pl
 
 func (d *Reconciler) generatePlan(ctx context.Context, cachedStatus inspectorInterface.Inspector, generators ...planGenerator) (error, bool) {
 	updated := false
+	updateRequired := false
 
 	if err := d.context.WithStatusUpdate(ctx, func(s *api.DeploymentStatus) bool {
 		var b api.BackOff
@@ -103,11 +104,11 @@ func (d *Reconciler) generatePlan(ctx context.Context, cachedStatus inspectorInt
 
 			if !new.Equal(s.BackOff) {
 				s.BackOff = new
-				updated = true
+				updateRequired = true
 			}
 		}
 
-		return updated
+		return updated || updateRequired
 	}); err != nil {
 		return errors.WithMessage(err, "Unable to save plan"), false
 	}
diff --git a/pkg/deployment/reconcile/plan_builder_license.go b/pkg/deployment/reconcile/plan_builder_license.go
@@ -26,6 +26,7 @@ import (
 	api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
 	"github.com/arangodb/kube-arangodb/pkg/deployment/client"
 	"github.com/arangodb/kube-arangodb/pkg/util/arangod"
+	"github.com/arangodb/kube-arangodb/pkg/util/globals"
 	"github.com/arangodb/kube-arangodb/pkg/util/k8sutil"
 	inspectorInterface "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector"
 	"github.com/rs/zerolog"
@@ -67,15 +68,18 @@ func updateClusterLicense(ctx context.Context,
 
 	member := members[0]
 
-	c, err := context.GetServerClient(ctx, member.Group, member.Member.ID)
+	ctxChild, cancel := globals.GetGlobals().Timeouts().ArangoD().WithTimeout(ctx)
+	defer cancel()
+
+	c, err := context.GetServerClient(ctxChild, member.Group, member.Member.ID)
 	if err != nil {
 		log.Err(err).Msgf("Unable to get client")
 		return nil
 	}
 
 	internalClient := client.NewClient(c.Connection())
 
-	if ok, err := licenseV2Compare(ctx, internalClient, l.V2); err != nil {
+	if ok, err := licenseV2Compare(ctxChild, internalClient, l.V2); err != nil {
 		log.Error().Err(err).Msg("Unable to verify license")
 		return nil
 	} else if ok {
diff --git a/pkg/deployment/resources/license.go b/pkg/deployment/resources/license.go
@@ -40,9 +40,19 @@ func (r *Resources) ValidateLicenseKeySecret(cachedStatus inspectorInterface.Ins
 			return errors.Newf("License secret %s does not exist", s)
 		}
 
-		if _, ok := s.Data[constants.SecretKeyToken]; !ok {
-			return errors.Newf("Invalid secret format")
+		if _, ok := s.Data[constants.SecretKeyToken]; ok {
+			return nil
 		}
+
+		if _, ok := s.Data[constants.SecretKeyV2Token]; ok {
+			return nil
+		}
+
+		if _, ok := s.Data[constants.SecretKeyV2License]; ok {
+			return nil
+		}
+
+		return errors.Newf("Invalid secret format")
 	}
 
 	return nil
diff --git a/pkg/logging/logger.go b/pkg/logging/logger.go
@@ -128,6 +128,8 @@ func (s *loggingService) MustSetLevel(name, level string) {
 // stringToLevel converts a level string to a zerolog level
 func stringToLevel(l string) (zerolog.Level, error) {
 	switch strings.ToLower(l) {
+	case "trace":
+		return zerolog.TraceLevel, nil
 	case "debug":
 		return zerolog.DebugLevel, nil
 	case "info":

Original file line number	Diff line number	Diff line change
`@@ -407,7 +407,7 @@ func (d *Deployment) handleArangoDeploymentUpdatedEvent(ctx context.Context) err`
`407`	`407`	`if err := newAPIObject.Spec.Validate(); err != nil {`
`408`	`408`	`d.CreateEvent(k8sutil.NewErrorEvent("Validation failed", err, d.apiObject))`
`409`	`409`	`// Try to reset object`
`410`		`- if err := d.updateCRSpec(ctx, d.apiObject.Spec, true); err != nil {`
	`410`	`+ if err := d.updateCRSpec(ctx, d.apiObject.Spec); err != nil {`
`411`	`411`	`log.Error().Err(err).Msg("Restore original spec failed")`
`412`	`412`	`d.CreateEvent(k8sutil.NewErrorEvent("Restore original failed", err, d.apiObject))`
`413`	`413`	`}`
`@@ -421,7 +421,7 @@ func (d *Deployment) handleArangoDeploymentUpdatedEvent(ctx context.Context) err`
`421`	`421`	`}`
`422`	`422`
`423`	`423`	`// Save updated spec`
`424`		`- if err := d.updateCRSpec(ctx, newAPIObject.Spec, true); err != nil {`
	`424`	`+ if err := d.updateCRSpec(ctx, newAPIObject.Spec); err != nil {`
`425`	`425`	`return errors.WithStack(errors.Newf("failed to update ArangoDeployment spec: %v", err))`
`426`	`426`	`}`
`427`	`427`	`// Save updated accepted spec`
Original file line number	Diff line number	Diff line change
`@@ -163,7 +163,7 @@ func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterva`
`163`	`163`	`} else {`
`164`	`164`	`condition, exists := status.Conditions.Get(api.ConditionTypeUpToDate)`
`165`	`165`	`if checksum != status.AppliedVersion && (!exists \|\| condition.IsTrue()) {`
`166`		`- if err = d.updateCondition(ctx, api.ConditionTypeUpToDate, false, "Spec Changed", "Spec Object changed. Waiting until plan will be applied"); err != nil {`
	`166`	`+ if err = d.updateConditionWithHash(ctx, api.ConditionTypeUpToDate, false, "Spec Changed", "Spec Object changed. Waiting until plan will be applied", checksum); err != nil {`
`167`	`167`	`return minInspectionInterval, errors.Wrapf(err, "Unable to update UpToDate condition")`
`168`	`168`	`}`
`169`	`169`
`@@ -265,9 +265,9 @@ func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterva`
`265`	`265`	`return minInspectionInterval, errors.Wrapf(err, "Unable clean plan")`
`266`	`266`	`}`
`267`	`267`	`} else if err, updated := d.reconciler.CreatePlan(ctx, cachedStatus); err != nil {`
`268`		`- d.deps.Log.Info().Msgf("Plan generated, reconciling")`
`269`	`268`	`return minInspectionInterval, errors.Wrapf(err, "Plan creation failed")`
`270`	`269`	`} else if updated {`
	`270`	`+ d.deps.Log.Info().Msgf("Plan generated, reconciling")`
`271`	`271`	`return minInspectionInterval, nil`
`272`	`272`	`}`
`273`	`273`
`@@ -284,15 +284,15 @@ func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterva`
`284`	`284`	`isUpToDate, reason := d.isUpToDateStatus()`
`285`	`285`
`286`	`286`	`if !isUpToDate && status.Conditions.IsTrue(api.ConditionTypeUpToDate) {`
`287`		`- if err = d.updateCondition(ctx, api.ConditionTypeUpToDate, false, reason, "There are pending operations in plan or members are in restart process"); err != nil {`
	`287`	`+ if err = d.updateConditionWithHash(ctx, api.ConditionTypeUpToDate, false, reason, "There are pending operations in plan or members are in restart process", checksum); err != nil {`
`288`	`288`	`return minInspectionInterval, errors.Wrapf(err, "Unable to update UpToDate condition")`
`289`	`289`	`}`
`290`	`290`
`291`	`291`	`return minInspectionInterval, nil`
`292`	`292`	`}`
`293`	`293`
`294`	`294`	`if isUpToDate && !status.Conditions.IsTrue(api.ConditionTypeUpToDate) {`
`295`		`- if err = d.updateCondition(ctx, api.ConditionTypeUpToDate, true, "Spec is Up To Date", "Spec is Up To Date"); err != nil {`
	`295`	`+ if err = d.updateConditionWithHash(ctx, api.ConditionTypeUpToDate, true, "Spec is Up To Date", "Spec is Up To Date", checksum); err != nil {`
`296`	`296`	`return minInspectionInterval, errors.Wrapf(err, "Unable to update UpToDate condition")`
`297`	`297`	`}`
`298`	`298`
`@@ -420,10 +420,10 @@ func (d *Deployment) triggerCRDInspection() {`
`420`	`420`	`d.inspectCRDTrigger.Trigger()`
`421`	`421`	`}`
`422`	`422`
`423`		`-func (d *Deployment) updateCondition(ctx context.Context, conditionType api.ConditionType, status bool, reason, message string) error {`
`424`		`- d.deps.Log.Info().Str("condition", string(conditionType)).Bool("status", status).Str("reason", reason).Str("message", message).Msg("Updated condition")`
	`423`	`+func (d *Deployment) updateConditionWithHash(ctx context.Context, conditionType api.ConditionType, status bool, reason, message, hash string) error {`
	`424`	`+ d.deps.Log.Info().Str("condition", string(conditionType)).Bool("status", status).Str("reason", reason).Str("message", message).Str("hash", hash).Msg("Updated condition")`
`425`	`425`	`if err := d.WithStatusUpdate(ctx, func(s *api.DeploymentStatus) bool {`
`426`		`- return s.Conditions.Update(conditionType, status, reason, message)`
	`426`	`+ return s.Conditions.UpdateWithHash(conditionType, status, reason, message, hash)`
`427`	`427`	`}); err != nil {`
`428`	`428`	`return errors.Wrapf(err, "Unable to update condition")`
`429`	`429`	`}`
Original file line number	Diff line number	Diff line change
`@@ -65,6 +65,7 @@ func (d *Reconciler) generatePlanFunc(gen planGeneratorFunc, planner planner) pl`
`65`	`65`
`66`	`66`	`func (d *Reconciler) generatePlan(ctx context.Context, cachedStatus inspectorInterface.Inspector, generators ...planGenerator) (error, bool) {`
`67`	`67`	`updated := false`
	`68`	`+ updateRequired := false`
`68`	`69`
`69`	`70`	`if err := d.context.WithStatusUpdate(ctx, func(s *api.DeploymentStatus) bool {`
`70`	`71`	`var b api.BackOff`
`@@ -103,11 +104,11 @@ func (d *Reconciler) generatePlan(ctx context.Context, cachedStatus inspectorInt`
`103`	`104`
`104`	`105`	`if !new.Equal(s.BackOff) {`
`105`	`106`	`s.BackOff = new`
`106`		`- updated = true`
	`107`	`+ updateRequired = true`
`107`	`108`	`}`
`108`	`109`	`}`
`109`	`110`
`110`		`- return updated`
	`111`	`+ return updated \|\| updateRequired`
`111`	`112`	`}); err != nil {`
`112`	`113`	`return errors.WithMessage(err, "Unable to save plan"), false`
`113`	`114`	`}`