fix(memorystore): prevent infinite reconciliation drift loop by aligning connections list length

[ldanielmadariaga]

Why is this change necessary?

During E2E testing of network connectivity topologies, we observed that MemorystoreInstance controllers can fall into an infinite reconciliation loop due to false drift detection on the endpoints[i].connections list.

Specifically:

• In the KRM spec (desired), a user defines the target VPC network(s) under spec.endpoints[i].connections (typically 1 connection item representing the intended PSC network target).
• Under the hood, GCP automatically allocates and returns multiple server-generated connection entries per target network (e.g., one connection per shard or node attachment).
• Because the GCP API returns an actual connections list that is longer than the user's desired list, tags.DiffForTopLevelFields flags the length mismatch as a drift. KCC repeatedly sends update requests attempting to shrink the connections list, which GCP ignores, resulting in an infinite reconciliation loop.

What does this change do?

1. Normalizes Connections list length before diffing (pkg/controller/direct/memorystore/memorystoreinstance_controller.go):
   Truncates the server-generated extra Connections in maskedActual to match len(desiredEndpoint.Connections) before comparison (actualEndpoint.Connections = actualEndpoint.Connections[:len(desiredEndpoint.Connections)]).
   • Why truncating is correct and does not cause data loss: Truncation only applies to the temporary maskedActual copy used for drift comparison against the user's spec. It does not discard any user-configured target connections, nor does it affect the full observed state reported in status.endpoints. It ensures we compare exactly the user-declared intent (desired) against the corresponding actual connection items without failing on extra server-created replica entries.
2. Prevents partial status overwrites (pkg/controller/direct/directbase/operations.go):
   Updates directbase/operations.go to prevent partial status updates from overwriting existing status fields (such as conditions or externalRef) with nil.
3. Sorts HTTP logs in E2E tests:
   Aligns/sorts HTTP log entries for the MemorystoreInstance test fixtures to prevent test flakiness.

[gemmahou]

Not a blocker of this PR.

This seems another use case for the issue I've experienced: The API converts the desired state into a value that we cannot easily simulate on our end, unlike predictable conversions we already handle(e.g., mapping project ID to number or altering string/enum casing, ordering, etc).

I am leaning toward a more generic solution that deserves a design discussion: storing a hash(last applied desired state, last applied actual state) in new field "ObservedState.LastApplied". We can then compare the current hash(desired state, actual state) against this stored hash to detect diffs. We should document the specific fields requiring this logic so we don't hash all fields unnecessarily.

cc @maqiuyujoyce

[ldanielmadariaga]

Yeah this makes sense, a nice thing in this case is that we do store the received values in status so we shouldn't lose this data

[maqiuyujoyce]

I think this is a different use case? The reason we want observedState to store the actual value for comparison is that the service-returned value and the user-provided value are different. And for this use case, the service still return the same values.

[gemmahou]

Agreed. The current logic to ensure desired is a subset of actual works to me.

[maqiuyujoyce]

/lgtm

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: acpana, maqiuyujoyce

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [acpana,maqiuyujoyce]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment