Read the veccgslb in the defaultgslbmanager#72
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends DefaultGslbManager to also retrieve legacy V1VeccGslb resources and return them alongside V1Gslb resources by converting V1VeccGslb into V1Gslb, supporting backward compatibility during migration.
Changes:
- Update
GetGslbsAsyncto list bothV1GslbandV1VeccGslband merge results. - Add a
ToV1Gslb(V1VeccGslb)conversion helper. - Update unit tests to expect
V1VeccGslbitems to be included in the returned array.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/Cyclops.MultiCluster/Services/Default/DefaultGslbManager.cs |
Lists legacy V1VeccGslb resources and converts them into V1Gslb for unified consumption. |
src/Cyclops.MultiCluster.Tests/Services/Default/DefaultGslbManagerTests.cs |
Expands test setup/assertions to include V1VeccGslb results in GetGslbsAsync. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,5 +1,6 @@ | |||
| using KubeOps.KubernetesClient; | |||
| using Cyclops.MultiCluster.Models.K8sEntities; | |||
| using k8s.Models; | |||
There was a problem hiding this comment.
using k8s.Models; appears unused in this file. Unused usings can produce build warnings (and can fail builds if warnings are treated as errors). Remove it unless something in this file explicitly needs types from k8s.Models.
| using k8s.Models; |
| ApiVersion = veccGslb.ApiVersion, | ||
| Kind = "Gslb", |
There was a problem hiding this comment.
ToV1Gslb is overriding ApiVersion and Kind with values that don’t match V1Gslb (it uses the legacy vecc API version and "Gslb" casing). This can cause the returned objects to be serialized/handled as the wrong CRD. Prefer leaving these unset so V1Gslb’s constructor defaults apply (or set them explicitly to the V1Gslb values).
| ApiVersion = veccGslb.ApiVersion, | |
| Kind = "Gslb", |
| Kind = veccGslb.ObjectReference.Kind == V1VeccGslb.V1ObjectReference.ReferenceType.Ingress | ||
| ? V1Gslb.V1ObjectReference.ReferenceType.Ingress | ||
| : V1Gslb.V1ObjectReference.ReferenceType.Service, | ||
| Name = veccGslb.Name(), |
There was a problem hiding this comment.
ObjectReference.Name is being set to veccGslb.Name() (the CRD object’s metadata.name) rather than the referenced ingress/service name. This will make downstream logic look up the wrong Service/Ingress. It should use veccGslb.ObjectReference.Name (matching the conversion logic already used in K8sChangedController).
| Name = veccGslb.Name(), | |
| Name = veccGslb.ObjectReference.Name, |
| [Fact] | ||
| public async Task GetGslbsAsync_ReturnsArray() | ||
| { | ||
| // Arrange | ||
| var gslb1 = new V1Gslb { Metadata = new k8s.Models.V1ObjectMeta { Name = "gslb1" } }; | ||
| var gslb2 = new V1Gslb { Metadata = new k8s.Models.V1ObjectMeta { Name = "gslb2" } }; | ||
| var veccGslb1 = new V1VeccGslb { Metadata = new k8s.Models.V1ObjectMeta { Name = "veccgslb1" } }; | ||
| var veccGslb2 = new V1VeccGslb { Metadata = new k8s.Models.V1ObjectMeta { Name = "veccgslb2" } }; | ||
|
|
||
| _clientMock.Setup(x => x.ListAsync<V1Gslb>(null)) | ||
| .ReturnsAsync(new List<V1Gslb> { gslb1, gslb2 }); | ||
| _clientMock.Setup(x => x.ListAsync<V1VeccGslb>(null)) | ||
| .ReturnsAsync(new List<V1VeccGslb> { veccGslb1, veccGslb2 }); | ||
|
|
||
| var manager = new DefaultGslbManager(_loggerMock.Object, _clientMock.Object); | ||
|
|
||
| // Act | ||
| var result = await manager.GetGslbsAsync(); | ||
|
|
||
| // Assert | ||
| Assert.Equal(2, result.Length); | ||
| Assert.Equal(4, result.Length); | ||
| Assert.Equal("gslb1", result[0].Metadata.Name); | ||
| Assert.Equal("gslb2", result[1].Metadata.Name); | ||
| Assert.Equal("veccgslb1", result[2].Metadata.Name); | ||
| Assert.Equal("veccgslb2", result[3].Metadata.Name); |
There was a problem hiding this comment.
The updated test only asserts that the converted V1VeccGslb items contribute metadata names, but it doesn’t verify the correctness of the conversion (e.g., Spec.ObjectReference.Name, Spec.ObjectReference.Kind, hostnames, etc.). Consider setting veccGslb*.ObjectReference.Name/Kind and asserting the resulting V1Gslb.Spec fields to prevent regressions in the ToV1Gslb mapping.
No description provided.