Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
randmonkey committed Nov 2, 2023
1 parent 7b784ad commit f92f126
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 9 deletions.
6 changes: 3 additions & 3 deletions internal/clients/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,12 @@ func (c *AdminAPIClientsManager) GatewayClientsToConfigure() []*adminapi.Client
c.lock.RLock()
defer c.lock.RUnlock()
readyGatewayClients := lo.Values(c.readyGatewayClients)
// With dbless mode, we should send configuration to ALL Kong gateway instances.
// With DB-less mode, we should send the configuration to ALL gateway instances.
if dataplaneutil.IsDBLessMode(c.dbMode) {
return readyGatewayClients
}
// When Kong gateway is DB backed, we return a random admin API client
// since KIC only need to send requests to one instance.
// When a gateway is DB-backed, we return a random client
// since KIC only needs to send requests to one instance.
// If there are no ready gateway clients, we return an empty list.
if len(readyGatewayClients) == 0 {
return []*adminapi.Client{}
Expand Down
3 changes: 0 additions & 3 deletions internal/clients/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,6 @@ func TestAdminAPIClientsManager_Clients_DBMode(t *testing.T) {
context.Background(),
zapr.NewLogger(zap.NewNop()),
initialClients,

&mockReadinessChecker{},
)
require.NoError(t, err)
Expand Down Expand Up @@ -252,7 +251,6 @@ func TestAdminAPIClientsManager_SubscribeToGatewayClientsChanges(t *testing.T) {
ctx,
zapr.NewLogger(zap.NewNop()),
[]*adminapi.Client{testClient},

readinessChecker)

require.NoError(t, err)
Expand Down Expand Up @@ -472,7 +470,6 @@ func TestAdminAPIClientsManager_PeriodicReadinessReconciliation(t *testing.T) {
ctx,
zapr.NewLogger(zap.NewNop()),
[]*adminapi.Client{testClient},

readinessChecker,
clients.WithReadinessReconciliationTicker(readinessTicker),
)
Expand Down
6 changes: 3 additions & 3 deletions internal/dataplane/kong_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -474,9 +474,9 @@ func (c *KongClient) sendOutToGatewayClients(
return nil, err
}

// After succeeded to update configurations in DB mode, we should propagate the SHA from
// the chosen gateway client to other clients.
if !dataplaneutil.IsDBLessMode(c.dbmode) && len(shas) > 0 {
// After a successful configuration update in DB mode, we should propagate the SHA from
// the chosen client to other clients as well as they will pick the configuration from the shared database.
if !dataplaneutil.IsDBLessMode(c.dbmode) && len(gatewayClients) > 1 {
for _, client := range gatewayClients {
client.SetLastConfigSHA([]byte(shas[0]))
}
Expand Down

4 comments on commit f92f126

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Go Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: f92f126 Previous: 2edf55e Ratio
BenchmarkDeckgenGenerateSHA - ns/op 85865 ns/op 29538 ns/op 2.91

This comment was automatically generated by workflow using github-action-benchmark.

CC: @Kong/k8s-maintainers

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Go Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: f92f126 Previous: 2edf55e Ratio
BenchmarkDeckgenGenerateSHA - ns/op 70221 ns/op 29538 ns/op 2.38

This comment was automatically generated by workflow using github-action-benchmark.

CC: @Kong/k8s-maintainers

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Go Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: f92f126 Previous: 2edf55e Ratio
BenchmarkDeckgenGenerateSHA - ns/op 67740 ns/op 29538 ns/op 2.29

This comment was automatically generated by workflow using github-action-benchmark.

CC: @Kong/k8s-maintainers

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Go Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: f92f126 Previous: 2edf55e Ratio
BenchmarkDeckgenGenerateSHA - ns/op 71566 ns/op 29538 ns/op 2.42

This comment was automatically generated by workflow using github-action-benchmark.

CC: @Kong/k8s-maintainers

Please sign in to comment.