Skip to content

Commit

Permalink
Added some clarifying comments, fixed watchable key deletion in the
Browse files Browse the repository at this point in the history
runner

Signed-off-by: Lior Okman <lior.okman@sap.com>
  • Loading branch information
liorokman committed May 12, 2024
1 parent 3146de9 commit b10c677
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 22 deletions.
33 changes: 13 additions & 20 deletions internal/gatewayapi/extensionserverpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,54 +42,47 @@ func (t *Translator) ProcessExtensionServerPolicies(policies []unstructured.Unst
gatewayMap[key] = &policyGatewayTargetContext{GatewayContext: gw}
}

// Process the policies targeting Gateways
// Process the policies targeting Gateways. Only update the policy status if it was accepted.
// A policy is considered accepted if at least one targetRef contained inside matched a listener.
for _, policy := range policies {
policy := policy.DeepCopy()
var policyStatus gwv1a2.PolicyStatus
res = append(res, *policy)
targetRefs, err := extractTargetRefs(policy)
if err != nil {
// TODO: mark as an error
t.Logger.Sugar().Errorf("error finding target refs: %w", err)
t.Logger.Sugar().Errorf("error finding targetRefs for policy %s: %w", policy.GetName(), err)
continue

Check warning on line 54 in internal/gatewayapi/extensionserverpolicy.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/extensionserverpolicy.go#L53-L54

Added lines #L53 - L54 were not covered by tests
}
for _, currTarget := range targetRefs {
if currTarget.Kind != KindGateway {
// TODO: mark this targetRef as an error
t.Logger.Sugar().Errorf("extension policy doesn't target a Gateway: %s", policy.GetName())
t.Logger.Sugar().Errorf("extension policy %s doesn't target a Gateway", policy.GetName())
continue

Check warning on line 60 in internal/gatewayapi/extensionserverpolicy.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/extensionserverpolicy.go#L58-L60

Added lines #L58 - L60 were not covered by tests
}

// Negative statuses have already been assigned so its safe to skip
gateway, resolveErr := resolveExtServerPolicyGatewayTargetRef(policy, currTarget, gatewayMap)
if gateway == nil {
t.Logger.Sugar().Infof("Continuing, gateway is nil")
t.Logger.Sugar().Warnf("unable to find a matching Gateway for policy %s", policy.GetName())
continue

Check warning on line 67 in internal/gatewayapi/extensionserverpolicy.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/extensionserverpolicy.go#L66-L67

Added lines #L66 - L67 were not covered by tests
}

// Find its ancestor reference by resolved gateway, even with resolve error
gatewayNN := utils.NamespacedName(gateway)
ancestorRefs := []gwv1a2.ParentReference{
getAncestorRefForPolicy(gatewayNN, currTarget.SectionName),
}

// Set conditions for resolve error, then skip current gateway
// Skip the gateway. Don't add anything to the policy status.
if resolveErr != nil {
t.Logger.Sugar().Info("resolveErr is not nil: %s", resolveErr)
status.SetResolveErrorForPolicyAncestors(&policyStatus,
ancestorRefs,
t.GatewayControllerName,
policy.GetGeneration(),
resolveErr,
)

t.Logger.Sugar().Warnf("targetRef %s in policy %s cannot be attached: %s",
currTarget, policy.GetName(), resolveErr)
continue

Check warning on line 74 in internal/gatewayapi/extensionserverpolicy.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/extensionserverpolicy.go#L72-L74

Added lines #L72 - L74 were not covered by tests
}

// Set conditions for translation if it got any
if t.translateExtServerPolicyForGateway(policy, gateway, currTarget, xdsIR) {
// Set Accepted condition if it is unset
// Only add a status condition if the policy was added into the IR
// Find its ancestor reference by resolved gateway, even with resolve error
gatewayNN := utils.NamespacedName(gateway)
ancestorRefs := []gwv1a2.ParentReference{
getAncestorRefForPolicy(gatewayNN, currTarget.SectionName),
}
status.SetAcceptedForPolicyAncestors(&policyStatus, ancestorRefs, t.GatewayControllerName)
}

Expand Down
7 changes: 7 additions & 0 deletions internal/gatewayapi/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,10 @@ func (r *Runner) deleteStatusKeys(ds *StatusesToDelete) {
r.ProviderResources.EnvoyExtensionPolicyStatuses.Delete(key)
delete(ds.EnvoyExtensionPolicyStatusKeys, key)
}
for key := range ds.ExtensionServerPolicyStatusKeys {
r.ProviderResources.ExtensionPolicyStatuses.Delete(key)
delete(ds.ExtensionServerPolicyStatusKeys, key)
}

Check warning on line 369 in internal/gatewayapi/runner/runner.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/runner/runner.go#L367-L369

Added lines #L367 - L369 were not covered by tests
}

// deleteAllStatusKeys deletes all status keys stored by the subscriber.
Expand Down Expand Up @@ -403,6 +407,9 @@ func (r *Runner) deleteAllStatusKeys() {
for key := range r.ProviderResources.EnvoyExtensionPolicyStatuses.LoadAll() {
r.ProviderResources.EnvoyExtensionPolicyStatuses.Delete(key)
}
for key := range r.ProviderResources.ExtensionPolicyStatuses.LoadAll() {
r.ProviderResources.ExtensionPolicyStatuses.Delete(key)
}

Check warning on line 412 in internal/gatewayapi/runner/runner.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/runner/runner.go#L411-L412

Added lines #L411 - L412 were not covered by tests
}

// getIRKeysToDelete returns the list of IR keys to delete
Expand Down
2 changes: 0 additions & 2 deletions internal/provider/kubernetes/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,6 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context) {
val := update.Value
obj := unstructured.Unstructured{}
obj.SetGroupVersionKind(key.GroupVersionKind)
r.log.Info("Called for updating an extension server policy status", "key", update.Key, "gvk", key.GroupVersionKind)

r.statusUpdater.Send(Update{
NamespacedName: key.NamespacedName,
Expand All @@ -427,7 +426,6 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context) {
}
tCopy := t.DeepCopy()
tCopy.Object["status"] = *val
r.log.Info("updating policy", "key", key)
return tCopy

Check warning on line 429 in internal/provider/kubernetes/status.go

View check run for this annotation

Codecov / codecov/patch

internal/provider/kubernetes/status.go#L427-L429

Added lines #L427 - L429 were not covered by tests
}),
})
Expand Down

0 comments on commit b10c677

Please sign in to comment.