Skip to content

Commit

Permalink
Merge pull request #2092 from gauravkghildiyal/alpha-neg-api
Browse files Browse the repository at this point in the history
Use Alpha NEG API when DualStackNEG feature is enabled
  • Loading branch information
k8s-ci-robot authored May 3, 2023
2 parents 8321bc1 + 5433728 commit 2d3d231
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 9 deletions.
5 changes: 4 additions & 1 deletion pkg/neg/syncers/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ type transactionSyncer struct {

// enableDegradedMode indicates whether we do endpoint calculation using degraded mode procedures
enableDegradedMode bool
// Enables support for Dual-Stack NEGs within the NEG Controller.
enableDualStackNEG bool

// podLabelPropagationConfig configures the pod label to be propagated to NEG endpoints
podLabelPropagationConfig labels.PodLabelPropagationConfig
Expand Down Expand Up @@ -168,6 +170,7 @@ func NewTransactionSyncer(
errorState: false,
logger: logger,
enableDegradedMode: flags.F.EnableDegradedMode,
enableDualStackNEG: enableDualStackNEG,
podLabelPropagationConfig: lpConfig,
networkInfo: networkInfo,
}
Expand Down Expand Up @@ -244,7 +247,7 @@ func (s *transactionSyncer) syncInternalImpl() error {
}
s.logger.V(2).Info("Sync NEG", "negSyncerKey", s.NegSyncerKey.String(), "endpointsCalculatorMode", s.endpointsCalculator.Mode())

currentMap, currentPodLabelMap, err := retrieveExistingZoneNetworkEndpointMap(s.NegSyncerKey.NegName, s.zoneGetter, s.cloud, s.NegSyncerKey.GetAPIVersion(), s.endpointsCalculator.Mode())
currentMap, currentPodLabelMap, err := retrieveExistingZoneNetworkEndpointMap(s.NegSyncerKey.NegName, s.zoneGetter, s.cloud, s.NegSyncerKey.GetAPIVersion(), s.endpointsCalculator.Mode(), s.enableDualStackNEG)
if err != nil {
return fmt.Errorf("%w: %v", negtypes.ErrCurrentNegEPNotFound, err)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/neg/syncers/transaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1540,7 +1540,7 @@ func TestUnknownNodes(t *testing.T) {
}

// Check that unknown zone did not cause endpoints to be removed
out, _, err := retrieveExistingZoneNetworkEndpointMap(testNegName, zoneGetter, fakeCloud, meta.VersionGA, negtypes.L7Mode)
out, _, err := retrieveExistingZoneNetworkEndpointMap(testNegName, zoneGetter, fakeCloud, meta.VersionGA, negtypes.L7Mode, false)
if err != nil {
t.Errorf("errored retrieving existing network endpoints")
}
Expand Down Expand Up @@ -1835,7 +1835,7 @@ func TestEnableDegradedMode(t *testing.T) {
(s.syncer.(*syncer)).stopped = false
tc.modify(s)

out, _, err := retrieveExistingZoneNetworkEndpointMap(tc.negName, zoneGetter, fakeCloud, meta.VersionGA, negtypes.L7Mode)
out, _, err := retrieveExistingZoneNetworkEndpointMap(tc.negName, zoneGetter, fakeCloud, meta.VersionGA, negtypes.L7Mode, false)
if err != nil {
t.Errorf("errored retrieving existing network endpoints")
}
Expand All @@ -1848,7 +1848,7 @@ func TestEnableDegradedMode(t *testing.T) {
t.Errorf("syncInternal returned %v, expected %v", err, tc.expectErr)
}
err = wait.PollImmediate(time.Second, 3*time.Second, func() (bool, error) {
out, _, err = retrieveExistingZoneNetworkEndpointMap(tc.negName, zoneGetter, fakeCloud, meta.VersionGA, negtypes.L7Mode)
out, _, err = retrieveExistingZoneNetworkEndpointMap(tc.negName, zoneGetter, fakeCloud, meta.VersionGA, negtypes.L7Mode, false)
if err != nil {
return false, nil
}
Expand Down
12 changes: 8 additions & 4 deletions pkg/neg/syncers/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ func podBelongsToService(pod *apiv1.Pod, service *apiv1.Service) error {
}

// retrieveExistingZoneNetworkEndpointMap lists existing network endpoints in the neg and return the zone and endpoints map
func retrieveExistingZoneNetworkEndpointMap(negName string, zoneGetter negtypes.ZoneGetter, cloud negtypes.NetworkEndpointGroupCloud, version meta.Version, mode negtypes.EndpointsCalculatorMode) (map[string]negtypes.NetworkEndpointSet, labels.EndpointPodLabelMap, error) {
func retrieveExistingZoneNetworkEndpointMap(negName string, zoneGetter negtypes.ZoneGetter, cloud negtypes.NetworkEndpointGroupCloud, version meta.Version, mode negtypes.EndpointsCalculatorMode, enableDualStackNEG bool) (map[string]negtypes.NetworkEndpointSet, labels.EndpointPodLabelMap, error) {
// Include zones that have non-candidate nodes currently. It is possible that NEGs were created in those zones previously and the endpoints now became non-candidates.
// Endpoints in those NEGs now need to be removed. This mostly applies to VM_IP_NEGs where the endpoints are nodes.
zones, err := zoneGetter.ListZones(utils.AllNodesPredicate)
Expand Down Expand Up @@ -627,6 +627,9 @@ func retrieveExistingZoneNetworkEndpointMap(negName string, zoneGetter negtypes.
if ne.NetworkEndpoint.Port != 0 {
newNE.Port = strconv.FormatInt(ne.NetworkEndpoint.Port, 10)
}
if enableDualStackNEG {
newNE.IPv6 = ne.NetworkEndpoint.Ipv6Address
}
zoneNetworkEndpointMap[zone].Insert(newNE)
endpointPodLabelMap[newNE] = ne.NetworkEndpoint.Annotations
}
Expand Down Expand Up @@ -655,9 +658,10 @@ func makeEndpointBatch(endpoints negtypes.NetworkEndpointSet, negType negtypes.N
return nil, fmt.Errorf("failed to decode endpoint port %v: %w", networkEndpoint, err)
}
cloudNetworkEndpoint := &composite.NetworkEndpoint{
Instance: networkEndpoint.Node,
IpAddress: networkEndpoint.IP,
Port: int64(portNum),
Instance: networkEndpoint.Node,
IpAddress: networkEndpoint.IP,
Ipv6Address: networkEndpoint.IPv6,
Port: int64(portNum),
}
if flags.F.EnableNEGLabelPropagation {
annotations, ok := endpointPodLabelMap[networkEndpoint]
Expand Down
2 changes: 1 addition & 1 deletion pkg/neg/syncers/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1055,7 +1055,7 @@ func TestRetrieveExistingZoneNetworkEndpointMap(t *testing.T) {
for _, tc := range testCases {
tc.mutate(negCloud)
// tc.mode of "" will result in the default node predicate being selected, which is ok for this test.
endpointSets, annotationMap, err := retrieveExistingZoneNetworkEndpointMap(negName, zoneGetter, negCloud, meta.VersionGA, tc.mode)
endpointSets, annotationMap, err := retrieveExistingZoneNetworkEndpointMap(negName, zoneGetter, negCloud, meta.VersionGA, tc.mode, false)

if tc.expectErr {
if err == nil {
Expand Down
16 changes: 16 additions & 0 deletions pkg/neg/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/ingress-gce/pkg/annotations"
"k8s.io/ingress-gce/pkg/flags"
"k8s.io/ingress-gce/pkg/network"
"k8s.io/ingress-gce/pkg/utils"
"k8s.io/ingress-gce/pkg/utils/namer"
Expand Down Expand Up @@ -287,6 +288,21 @@ func (key NegSyncerKey) String() string {
// GetAPIVersion returns the compute API version to be used in order
// to create the negType specified in the given NegSyncerKey.
func (key NegSyncerKey) GetAPIVersion() meta.Version {
if flags.F.EnableDualStackNEG {
// This condition can be removed when we're getting rid of the feature flag
// or when GCE meta.VersionGA API supports the IPv6 fields.
//
// As an exception, it's easier here to access the global flag value and not
// plumb it into the NegSyncerKey. Generally, code within the NEG controller
// tires to avoid accessing global flag values to aid with scenarios in unit
// testing -- in this case though, the actual differentiator between Alpha
// and other versions is NOT something that can (or rather should) be
// covered within unit tests.
//
// TODO(gauravkghildiyal): Start using Beta APIs once they have the
// necessary changes.
return meta.VersionAlpha
}
switch key.NegType {
case NonGCPPrivateEndpointType:
return meta.VersionAlpha
Expand Down

0 comments on commit 2d3d231

Please sign in to comment.