endpoint: Default regeneration reason to "Invalid"

Force explicit initialization of regeneration reason to avoid defaulting to regeneration without datapath. Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
cilium · Apr 14, 2020 · 5478154 · 5478154
1 parent ce260e1
commit 5478154
Show file tree

Hide file tree

Showing 8 changed files with 41 additions and 12 deletions.
diff --git a/daemon/cmd/policy.go b/daemon/cmd/policy.go
@@ -68,7 +68,10 @@ func (d *Daemon) policyUpdateTrigger(reasons []string) {
 	log.Debugf("Regenerating all endpoints")
 	reason := strings.Join(reasons, ", ")
 
-	regenerationMetadata := &regeneration.ExternalRegenerationMetadata{Reason: reason}
+	regenerationMetadata := &regeneration.ExternalRegenerationMetadata{
+		Reason:            reason,
+		RegenerationLevel: regeneration.RegenerateWithoutDatapath,
+	}
 	d.endpointManager.RegenerateAllEndpoints(regenerationMetadata)
 }
 
@@ -461,7 +464,10 @@ func reactToRuleUpdates(epsToBumpRevision, epsToRegen *policy.EndpointSet, rev u
 	})
 
 	// Regenerate all other endpoints.
-	regenMetadata := &regeneration.ExternalRegenerationMetadata{Reason: "policy rules added"}
+	regenMetadata := &regeneration.ExternalRegenerationMetadata{
+		Reason:            "policy rules added",
+		RegenerationLevel: regeneration.RegenerateWithoutDatapath,
+	}
 	epsToRegen.ForEachGo(&enqueueWaitGroup, func(ep policy.Endpoint) {
 		if ep != nil {
 			switch e := ep.(type) {

diff --git a/daemon/cmd/policy_test.go b/daemon/cmd/policy_test.go
@@ -71,7 +71,8 @@ var (
 	testEndpointID = uint16(1)
 
 	regenerationMetadata = &regeneration.ExternalRegenerationMetadata{
-		Reason: "test",
+		Reason:            "test",
+		RegenerationLevel: regeneration.RegenerateWithoutDatapath,
 	}
 
 	CNPAllowTCP80 = api.PortRule{

diff --git a/pkg/endpoint/endpoint.go b/pkg/endpoint/endpoint.go
@@ -833,7 +833,8 @@ func (e *Endpoint) Update(cfg *models.EndpointConfigurationSpec) error {
 	// Note: This "retry" behaviour is better suited to a controller, and can be
 	// moved there once we have an endpoint regeneration controller.
 	regenCtx := &regeneration.ExternalRegenerationMetadata{
-		Reason: "endpoint was updated via API",
+		Reason:            "endpoint was updated via API",
+		RegenerationLevel: regeneration.RegenerateWithoutDatapath,
 	}
 
 	// If configuration options are provided, we only regenerate if necessary.
@@ -1025,7 +1026,10 @@ func (e *Endpoint) leaveLocked(proxyWaitGroup *completion.WaitGroup, conf Delete
 // RegenerateWait should only be called when endpoint's state has successfully
 // been changed to "waiting-to-regenerate"
 func (e *Endpoint) RegenerateWait(reason string) error {
-	if !<-e.Regenerate(&regeneration.ExternalRegenerationMetadata{Reason: reason}) {
+	if !<-e.Regenerate(&regeneration.ExternalRegenerationMetadata{
+		Reason:            reason,
+		RegenerationLevel: regeneration.RegenerateWithDatapathRewrite,
+	}) {
 		return fmt.Errorf("error while regenerating endpoint."+
 			" For more info run: 'cilium endpoint get %d'", e.ID)
 	}
@@ -1794,7 +1798,10 @@ func (e *Endpoint) identityLabelsChanged(ctx context.Context, myChangeRev int) e
 	e.unlock()
 
 	if readyToRegenerate {
-		e.Regenerate(&regeneration.ExternalRegenerationMetadata{Reason: "updated security labels"})
+		e.Regenerate(&regeneration.ExternalRegenerationMetadata{
+			Reason:            "updated security labels",
+			RegenerationLevel: regeneration.RegenerateWithDatapathRewrite,
+		})
 	}
 
 	return nil
@@ -2143,8 +2150,9 @@ func (e *Endpoint) RegenerateAfterCreation(ctx context.Context, syncBuild bool)
 		// proxy configuration, this code would effectively deadlock addition
 		// of endpoints.
 		e.Regenerate(&regeneration.ExternalRegenerationMetadata{
-			Reason:        "Initial build on endpoint creation",
-			ParentContext: ctx,
+			Reason:            "Initial build on endpoint creation",
+			ParentContext:     ctx,
+			RegenerationLevel: regeneration.RegenerateWithDatapathRewrite,
 		})
 	}
 

diff --git a/pkg/endpoint/regeneration/regeneration_context.go b/pkg/endpoint/regeneration/regeneration_context.go
@@ -23,9 +23,12 @@ import (
 type DatapathRegenerationLevel int
 
 const (
+	// Invalid is the default level to enforce explicit setting of
+	// the regeneration level.
+	Invalid DatapathRegenerationLevel = iota
 	// RegenerateWithoutDatapath indicates that datapath rebuild or reload
 	// is not required to implement this regeneration.
-	RegenerateWithoutDatapath DatapathRegenerationLevel = iota
+	RegenerateWithoutDatapath
 	// RegenerateWithDatapathLoad indicates that the datapath must be
 	// reloaded but not recompiled to implement this regeneration.
 	RegenerateWithDatapathLoad
@@ -40,6 +43,8 @@ const (
 // String converts a DatapathRegenerationLevel into a human-readable string.
 func (r DatapathRegenerationLevel) String() string {
 	switch r {
+	case Invalid:
+		return "invalid"
 	case RegenerateWithoutDatapath:
 		return "no-rebuild"
 	case RegenerateWithDatapathLoad:

diff --git a/pkg/endpoint/regenerationcontext.go b/pkg/endpoint/regenerationcontext.go
@@ -19,6 +19,7 @@ import (
 
 	"github.com/cilium/cilium/pkg/completion"
 	"github.com/cilium/cilium/pkg/endpoint/regeneration"
+	"github.com/cilium/cilium/pkg/logging/logfields"
 	"github.com/cilium/cilium/pkg/revert"
 )
 
@@ -46,6 +47,10 @@ type regenerationContext struct {
 }
 
 func ParseExternalRegenerationMetadata(ctx context.Context, c context.CancelFunc, e *regeneration.ExternalRegenerationMetadata) *regenerationContext {
+	if e.RegenerationLevel == regeneration.Invalid {
+		log.WithField(logfields.Reason, e.Reason).Errorf("Uninitialized regeneration level")
+	}
+
 	return &regenerationContext{
 		Reason: e.Reason,
 		datapathRegenerationContext: &datapathRegenerationContext{

diff --git a/pkg/endpoint/restore.go b/pkg/endpoint/restore.go
@@ -166,7 +166,8 @@ func (e *Endpoint) RegenerateAfterRestore() error {
 	scopedLog := log.WithField(logfields.EndpointID, e.ID)
 
 	regenerationMetadata := &regeneration.ExternalRegenerationMetadata{
-		Reason: "syncing state to host",
+		Reason:            "syncing state to host",
+		RegenerationLevel: regeneration.RegenerateWithDatapathRewrite,
 	}
 	if buildSuccess := <-e.Regenerate(regenerationMetadata); !buildSuccess {
 		scopedLog.Warn("Failed while regenerating endpoint")

diff --git a/pkg/endpoint/restore_test.go b/pkg/endpoint/restore_test.go
@@ -90,7 +90,8 @@ func (ds *EndpointSuite) endpointCreator(id uint16, secID identity.NumericIdenti
 
 var (
 	regenerationMetadata = &regeneration.ExternalRegenerationMetadata{
-		Reason: "test",
+		Reason:            "test",
+		RegenerationLevel: regeneration.RegenerateWithoutDatapath,
 	}
 )
 

diff --git a/test/helpers/cons.go b/test/helpers/cons.go
@@ -215,7 +215,8 @@ const (
 	logBufferMessage   = "Log buffer too small to dump verifier log"  // from https://github.com/cilium/cilium/issues/10517
 	ClangErrorsMsg     = " errors generated."                         // from https://github.com/cilium/cilium/issues/10857
 	ClangErrorMsg      = "1 error generated."                         // from https://github.com/cilium/cilium/issues/10857
-	symbolSubstitution = "Skipping symbol substitution"
+	symbolSubstitution = "Skipping symbol substitution"               //
+	uninitializedRegen = "Uninitialized regeneration level"           // from https://github.com/cilium/cilium/pull/10949
 
 	// HelmTemplate is the location of the Helm templates to install Cilium
 	HelmTemplate = "../install/kubernetes/cilium"
@@ -274,6 +275,7 @@ var badLogMessages = map[string][]string{
 	ClangErrorsMsg:     nil,
 	ClangErrorMsg:      nil,
 	symbolSubstitution: nil,
+	uninitializedRegen: nil,
 }
 
 var ciliumCLICommands = map[string]string{