Skip to content

Commit

Permalink
wip: diff diagnostic endpoint
Browse files Browse the repository at this point in the history
  • Loading branch information
rainest committed Jun 4, 2024
1 parent bc75c4f commit e16b328
Show file tree
Hide file tree
Showing 6 changed files with 145 additions and 2 deletions.
1 change: 1 addition & 0 deletions internal/dataplane/kong_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,7 @@ func (c *KongClient) sendToClient(
// apply the configuration update in Kong
timedCtx, cancel := context.WithTimeout(ctx, c.requestTimeout)
defer cancel()
// TRR this is the point in kongclient where db events maybe happen, but it's not differentiated by strategy here
newConfigSHA, err := sendconfig.PerformUpdate(
timedCtx,
logger,
Expand Down
49 changes: 47 additions & 2 deletions internal/dataplane/sendconfig/dbmode.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/kong/go-kong/kong"

"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/deckerrors"
"github.com/kong/kubernetes-ingress-controller/v3/internal/diagnostics"
"github.com/kong/kubernetes-ingress-controller/v3/internal/metrics"
"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
)
Expand Down Expand Up @@ -89,7 +90,11 @@ func (s UpdateStrategyDBMode) Update(ctx context.Context, targetContent ContentW
}

ctx, cancel := context.WithCancel(ctx)
go s.HandleEvents(ctx, syncer.GetResultChan())
// TRR this is where db mode update strat handles events. resultchan is the entityaction channel
// TRR targetContent.Hash is the config hash
// TRR TODO need to plumb the actual channel from the diag server over here. standin black hole for now
diffs := make(chan diagnostics.ConfigDiff, 3)
go s.HandleEvents(ctx, syncer.GetResultChan(), diffs, string(targetContent.Hash))

_, errs, _ := syncer.Solve(ctx, s.concurrency, false, false)
cancel()
Expand All @@ -115,10 +120,50 @@ func (s UpdateStrategyDBMode) Update(ctx context.Context, targetContent ContentW
return nil
}

// TRR upstream type
//// EntityAction describes an entity processed by the diff engine and the action taken on it.
//type EntityAction struct {
// // Action is the ReconcileAction taken on the entity.
// Action ReconcileAction `json:"action"` // string
// // Entity holds the processed entity.
// Entity Entity `json:"entity"`
// // Diff is diff string describing the modifications made to an entity.
// Diff string `json:"-"`
// // Error is the error encountered processing and entity, if any.
// Error error `json:"error,omitempty"`
//}
//
//// Entity is an entity processed by the diff engine.
//type Entity struct {
// // Name is the name of the entity.
// Name string `json:"name"`
// // Kind is the type of entity.
// Kind string `json:"kind"`
// // Old is the original entity in the current state, if any.
// Old any `json:"old,omitempty"`
// // New is the new entity in the target state, if any.
// New any `json:"new,omitempty"`
//}

// HandleEvents handles logging and error reporting for individual entity change events generated during a sync by
// looping over an event channel. It terminates when its context dies.
func (s *UpdateStrategyDBMode) HandleEvents(ctx context.Context, events chan diff.EntityAction) {
func (s *UpdateStrategyDBMode) HandleEvents(
ctx context.Context,
events chan diff.EntityAction,
diffChan chan diagnostics.ConfigDiff,
hash string,
) {
// TRR this is where we get the diff info from deck
s.resourceErrorLock.Lock()
// TRR TODO this accumulator isn't great since we need to append to the array, which is... probably unsafe? maybe
// the for can't actually handle multiple select inbounds at once, but I think it can, and append calls would cause
// havoc. can maybe use something from https://pkg.go.dev/sync/atomic to increment a counter, use that as a map key
// and then convert the values into a slice for the Done handler. otherwise this would need to send individual
// EntityDiffs down a channel to the diag server, which seems less than ideal
diff := diagnostics.ConfigDiff{

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / Run Go benchmarks

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / kongintegration-tests / kongintegration-tests

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / kongintegration-tests / kongintegration-tests

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / conformance-tests / traditional-compatible-router

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / integration-tests / isolated-postgres

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / envtest-tests / envtest-tests

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / envtest-tests / envtest-tests

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / conformance-tests / expressions-router

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / integration-tests / isolated-dbless

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / integration-tests / isolated-dbless-expression-router

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / integration-tests / enterprise-dbless

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / integration-tests / postgres-rewrite-uris

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / integration-tests / dbless

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / integration-tests / dbless-traditional-compatible

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / integration-tests / dbless-rewrite-uris

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / integration-tests / postgres

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / integration-tests / dbless-traditional

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / integration-tests / postgres-traditional

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / integration-tests / dbless-invalid-config

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / unit-tests / unit-tests

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / unit-tests / unit-tests

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / integration-tests / enterprise-postgres

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / integration-tests / dbless-fallback-config

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / integration-tests / postgres-fallback-config

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / integration-tests / dbless-gateway-alpha

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / integration-tests / postgres-gateway-alpha

diff declared and not used

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / linters / lint

diff declared and not used) (typecheck)

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / linters / lint

diff declared and not used) (typecheck)

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / linters / lint

diff declared and not used (typecheck)

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / linters / lint

diff declared and not used) (typecheck)

Check failure on line 163 in internal/dataplane/sendconfig/dbmode.go

View workflow job for this annotation

GitHub Actions / linters / lint

diff declared and not used) (typecheck)
Hash: hash,
Entities: []diagnostics.EntityDiff{},
}
for {
select {
case event := <-events:
Expand Down
1 change: 1 addition & 0 deletions internal/dataplane/sendconfig/sendconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ func PerformUpdate(
updateStrategy := updateStrategyResolver.ResolveUpdateStrategy(client)
logger = logger.WithValues("update_strategy", updateStrategy.Type())
timeStart := time.Now()
// TRR this is the point in sendconfig where db events maybe happen, but it's not differentiated by strategy here
err = updateStrategy.Update(ctx, ContentWithHash{
Content: targetContent,
Hash: newSHA,
Expand Down
93 changes: 93 additions & 0 deletions internal/diagnostics/diff.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package diagnostics

// TRR TODO this holds guts for diff endpoints. need to reorg the types.go into something that splits out the
// subcomponents into separate files and move the base stuff into server.go probably.
// the /config/{successful|failed} API endpoint structure isn't great for anything other than "current failure, last
// succeeded" state (which is something we still care about). simplest option to handle the diffs within that landscape
// is to have /config/{successful|failed}/diff, but that's a bit stuck on associating the correct one via the hashes.
// we do now _have_ the hashes, but getting the config and diff separate and linking them together after in the
// success/fail structure is maybe a bit wonky. a chronological sequence where the latest maybe has a failure attempt
// attached is maybe simpler, but doesn't _quite_ solve the binding problem--would need to convert the hashes into
// counters somehow

// TRR upstream type
//// EntityAction describes an entity processed by the diff engine and the action taken on it.
//type EntityAction struct {
// // Action is the ReconcileAction taken on the entity.
// Action ReconcileAction `json:"action"` // string
// // Entity holds the processed entity.
// Entity Entity `json:"entity"`
// // Diff is diff string describing the modifications made to an entity.
// Diff string `json:"-"`
// // Error is the error encountered processing and entity, if any.
// Error error `json:"error,omitempty"`
//}
//
//// Entity is an entity processed by the diff engine.
//type Entity struct {
// // Name is the name of the entity.
// Name string `json:"name"`
// // Kind is the type of entity.
// Kind string `json:"kind"`
// // Old is the original entity in the current state, if any.
// Old any `json:"old,omitempty"`
// // New is the new entity in the target state, if any.
// New any `json:"new,omitempty"`
//}

type sourceResource struct {
Name string
Namespace string
GroupVersionKind string
UID string
}

type generatedEntity struct {
// Name is the name of the entity.
Name string `json:"name"`
// Kind is the type of entity.
Kind string `json:"kind"`
}

type ConfigDiff struct {
Hash string `json:"hash"`
Entities []EntityDiff `json:"entities"`
}

type EntityDiff struct {
Source sourceResource `json:"kubernetesResource"`
Generated generatedEntity `json:"kongEntity"`
Action string `json:"action"`
Diff string `json:"diff,omitempty"`
}

// TRR TODO this is stolen from the error event builder, which parses regurgitated entity tags into k8s parents.
// we want the same here, minus the additional error info. could probably make it a function in
// internal/util/k8s.go along with sourceResource, but for now it's just sitting here for reference.
// the end function probably won't keep the GVK entries separate? dunno--we want that for the event builder, but for
// the diag server we just want to get the string version. can probably use the upstream GVK type in the generic
// function and call its String().

//func parseTags([]string) {
// for _, tag := range raw.Tags {
// if strings.HasPrefix(tag, util.K8sNameTagPrefix) {
// re.Name = strings.TrimPrefix(tag, util.K8sNameTagPrefix)
// }
// if strings.HasPrefix(tag, util.K8sNamespaceTagPrefix) {
// re.Namespace = strings.TrimPrefix(tag, util.K8sNamespaceTagPrefix)
// }
// if strings.HasPrefix(tag, util.K8sKindTagPrefix) {
// gvk.Kind = strings.TrimPrefix(tag, util.K8sKindTagPrefix)
// }
// if strings.HasPrefix(tag, util.K8sVersionTagPrefix) {
// gvk.Version = strings.TrimPrefix(tag, util.K8sVersionTagPrefix)
// }
// // this will not set anything for core resources
// if strings.HasPrefix(tag, util.K8sGroupTagPrefix) {
// gvk.Group = strings.TrimPrefix(tag, util.K8sGroupTagPrefix)
// }
// if strings.HasPrefix(tag, util.K8sUIDTagPrefix) {
// re.UID = strings.TrimPrefix(tag, util.K8sUIDTagPrefix)
// }
// }
//}
1 change: 1 addition & 0 deletions internal/diagnostics/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func NewServer(logger logr.Logger, cfg ServerConfig) Server {
s.configDumps = ConfigDumpDiagnostic{
DumpsIncludeSensitive: cfg.DumpSensitiveConfig,
Configs: make(chan ConfigDump, diagnosticConfigBufferDepth),
Diffs: make(chan ConfigDiff, diagnosticConfigBufferDepth),
}
}

Expand Down
2 changes: 2 additions & 0 deletions internal/diagnostics/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ type ConfigDumpDiagnostic struct {
DumpsIncludeSensitive bool
// Configs is the channel that receives configuration blobs from the configuration update strategy implementation.
Configs chan ConfigDump
// Diffs is the channel that receives diff info in DB mode.
Diffs chan ConfigDiff
}

// AffectedObject is a Kubernetes object associated with diagnostic information.
Expand Down

0 comments on commit e16b328

Please sign in to comment.