[Feature] Add Gateway Config condition #1959
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds gateway configuration monitoring by implementing a new condition to track the configuration state of Gateway members. The feature introduces a systematic way to monitor whether gateway configurations are present and up-to-date by comparing configuration checksums.
Key changes include:
- Addition of a new
ConditionTypeGatewayConfigcondition type for tracking gateway configuration state - Implementation of gateway configuration monitoring in the reconciler plan builder
- Restructuring of gateway configuration generation to calculate checksums after all destinations are configured
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/deployment/reconcile/plan_builder_gateway.go | New file implementing gateway config condition monitoring logic |
| pkg/deployment/client/inventory.go | New inventory client for fetching gateway configuration data |
| pkg/apis/deployment/v1/conditions.go | Adds the new GatewayConfig condition type constant |
| pkg/deployment/resources/config_map_gateway.go | Reorders configuration generation to calculate checksum after all destinations |
| pkg/deployment/reconcile/plan_builder_high.go | Integrates gateway config condition into the reconciliation plan |
| pkg/deployment/reconcile/plan_builder_context.go | Updates interface to use DeploymentGetter instead of DeploymentInfoGetter |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
There was a problem hiding this comment.
The variable logger is undefined in this scope. You need to either pass a logger parameter to this function or use a logger from the reconciler context.
| logger := log.FromContext(ctx) |
Copilot uses AI. Check for mistakes.
| } | ||
|
|
||
| if c, ok := m.Member.Conditions.Get(api.ConditionTypeGatewayConfig); !ok || c.Status == core.ConditionFalse || c.Hash != inv.GetConfiguration().GetHash() { | ||
| plan = append(plan, sharedReconcile.UpdateMemberConditionActionV2("Config Present", api.ConditionTypeGatewayConfig, m.Group, m.Member.ID, false, "Config Present", "Config Present", inv.GetConfiguration().GetHash())) |
There was a problem hiding this comment.
The hardcoded string "Config Present" is repeated multiple times. Consider defining a constant for this message to improve maintainability.
| plan = append(plan, sharedReconcile.UpdateMemberConditionActionV2("Config Present", api.ConditionTypeGatewayConfig, m.Group, m.Member.ID, false, "Config Present", "Config Present", inv.GetConfiguration().GetHash())) | |
| plan = append(plan, sharedReconcile.UpdateMemberConditionActionV2(gatewayConfigPresentMsg, api.ConditionTypeGatewayConfig, m.Group, m.Member.ID, false, gatewayConfigPresentMsg, gatewayConfigPresentMsg, inv.GetConfiguration().GetHash())) |
Copilot uses AI. Check for mistakes.
| inv, err := r.getGatewayInventoryConfig(ctx, planCtx, m.Group, m.Member) | ||
| if err != nil { | ||
| if c, ok := m.Member.Conditions.Get(api.ConditionTypeGatewayConfig); !ok || c.Status == core.ConditionTrue { | ||
| plan = append(plan, sharedReconcile.UpdateMemberConditionActionV2("Config is not present", api.ConditionTypeGatewayConfig, m.Group, m.Member.ID, false, "Config is not present", "Config is not present", "")) |
There was a problem hiding this comment.
The hardcoded string "Config is not present" is repeated multiple times. Consider defining a constant for this message to improve maintainability.
| plan = append(plan, sharedReconcile.UpdateMemberConditionActionV2("Config is not present", api.ConditionTypeGatewayConfig, m.Group, m.Member.ID, false, "Config is not present", "Config is not present", "")) | |
| plan = append(plan, sharedReconcile.UpdateMemberConditionActionV2(configNotPresentMsg, api.ConditionTypeGatewayConfig, m.Group, m.Member.ID, false, configNotPresentMsg, configNotPresentMsg, "")) |
Copilot uses AI. Check for mistakes.
| } | ||
|
|
||
| if c, ok := m.Member.Conditions.Get(api.ConditionTypeGatewayConfig); !ok || c.Status == core.ConditionFalse || c.Hash != inv.GetConfiguration().GetHash() { | ||
| plan = append(plan, sharedReconcile.UpdateMemberConditionActionV2("Config Present", api.ConditionTypeGatewayConfig, m.Group, m.Member.ID, false, "Config Present", "Config Present", inv.GetConfiguration().GetHash())) |
There was a problem hiding this comment.
The fourth parameter should be true instead of false when the config is present, as this indicates the condition status (true = condition is met).
| plan = append(plan, sharedReconcile.UpdateMemberConditionActionV2("Config Present", api.ConditionTypeGatewayConfig, m.Group, m.Member.ID, false, "Config Present", "Config Present", inv.GetConfiguration().GetHash())) | |
| plan = append(plan, sharedReconcile.UpdateMemberConditionActionV2("Config Present", api.ConditionTypeGatewayConfig, m.Group, m.Member.ID, true, "Config Present", "Config Present", inv.GetConfiguration().GetHash())) |
Copilot uses AI. Check for mistakes.
2 participants
No description provided.