Modify NEG libraries to use composite types. #926
Conversation
e42d748 to
35a5060
Compare
|
@spencerhance |
| @@ -53,6 +54,14 @@ type ServicePort struct { | |||
| BackendNamer namer.BackendNamer | |||
| } | |||
|
|
|||
| func GetAPIVersionFromServicePort(sp *ServicePort) meta.Version { | |||
There was a problem hiding this comment.
I think you can use VersionFromServicePort() in pkg/backends/features/features.go
There was a problem hiding this comment.
Synced offline to discuss this. I will add a TODO to unify this with the other version lookups we are doing. Current logic for api lookup is not on a per-resource basis.
|
The unit tests are still broken. Let me know if you need help to convert the unit tests to use composite types. |
35a5060 to
591f9fb
Compare
591f9fb to
190a06f
Compare
190a06f to
9aadc88
Compare
| } | ||
|
|
||
| return compositeMap, nil |
There was a problem hiding this comment.
I am thinking returning a map[meta.Key]*NetworkEndpoinGroup.
This is probably more natural than the current output.
Need a small change on the consumer side. I think only NEG GC uses AggregatedList.
| @@ -53,6 +54,14 @@ type ServicePort struct { | |||
| BackendNamer namer.BackendNamer | |||
| } | |||
|
|
|||
| func GetAPIVersionFromServicePort(sp *ServicePort) meta.Version { | |||
| @@ -383,7 +384,11 @@ func TestPoll(t *testing.T) { | |||
|
|
|||
| // add NE with healthy status | |||
| negtypes.GetNetworkEndpointStore(negCloud).AddNetworkEndpointHealthStatus(*meta.ZonalKey(negName, zone), negtypes.NetworkEndpointEntry{ | |||
| NetworkEndpoint: ne, | |||
| NetworkEndpoint: &compute.NetworkEndpoint{ | |||
There was a problem hiding this comment.
nit: is there a conversion helper function can change composite.NetworkEndpoint to compute.NetworkEndpoint?
There was a problem hiding this comment.
we do not generate it for derived resources - i.e There is one for NetworkEndpointGroup, NetworkEndpoint is created because it is part of the NetworkEndpointGroup proto. Given that we want to move the mock NetworkEndpointStore to also use composite types, I think it is ok to leave it for now. Is that ok?
| @@ -252,5 +255,14 @@ func (key NegSyncerKey) String() string { | |||
| return fmt.Sprintf("%s/%s-%s-%s", key.Namespace, key.Name, key.Subset, key.PortTuple.String()) | |||
| } | |||
|
|
|||
| func (key NegSyncerKey) GetAPIVersion() meta.Version { | |||
Modified syncer_test to use only one v1 import.
There is some code duplication since map[string]Struct cannot be cast
to map[string]interface{}
Fixed a bug in ListNetworkEndpointGroups, where the incorrect err
variable was being returned.
9aadc88 to
3849ae4
Compare
| ctx, cancel := cloud.ContextWithCallTimeout() | ||
| defer cancel() | ||
|
|
||
| func (a *cloudProviderAdapter) AggregatedListNetworkEndpointGroup(version meta.Version) (map[string][]*composite.NetworkEndpointGroup, error) { |
There was a problem hiding this comment.
change the adapter to return map[meta.Key]*composite.NetworkEndpointGroup as well?
Or add a TODO? Should not be too complex. The GC logic may need to change a little
There was a problem hiding this comment.
I added a TODO to my current branch, which has changes to create VM_IP NEGs. I will include this change along with that PR.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: freehan, prameshj The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Verified that the existing NEG creation/endpoint add works.