diff --git a/Makefile b/Makefile index bca5c20a79..f13cc98563 100644 --- a/Makefile +++ b/Makefile @@ -500,11 +500,11 @@ COVER_PKG ?= . test-all: ## run all unit tests. @$(eval COVER_FILTER=`go list --tags ignore_uncovered,ignore_autogenerated $(COVER_PKG)/... | tr '\n' ','`) @echo Test coverpkg: $(COVER_FILTER) - go test -buildvcs=false -tags "unit" -coverpkg=$(COVER_FILTER) -v -race -covermode atomic -failfast -coverprofile=coverage.out $(COVER_PKG)/... + go test -buildvcs=false -tags "unit" -coverpkg=$(COVER_FILTER) -race -covermode atomic -failfast -coverprofile=coverage.out $(COVER_PKG)/... test-integration: ## run all integration tests. - go test -buildvcs=false -timeout 1h -coverpkg=./... -v -race -covermode atomic -coverprofile=coverage.out -tags=integration ./test/integration... + go test -buildvcs=false -timeout 1h -coverpkg=./... -race -covermode atomic -coverprofile=coverage.out -tags=integration ./test/integration... test-cyclonus: ## run the cyclonus test for npm. cd test/cyclonus && bash ./test-cyclonus.sh diff --git a/cns/client/client_test.go b/cns/client/client_test.go index fb241c9e21..ab4cace1ce 100644 --- a/cns/client/client_test.go +++ b/cns/client/client_test.go @@ -97,7 +97,7 @@ func addTestStateToRestServer(t *testing.T, secondaryIps []string) { t.Fatalf("Failed to createNetworkContainerRequest, req: %+v, err: %d", req, returnCode) } - svc.IPAMPoolMonitor.Update(&v1alpha.NodeNetworkConfig{ + _ = svc.IPAMPoolMonitor.Update(&v1alpha.NodeNetworkConfig{ Spec: v1alpha.NodeNetworkConfigSpec{ RequestedIPCount: 16, IPsNotInUse: []string{"abc"}, diff --git a/cns/fakes/requestcontrollerfake.go b/cns/fakes/requestcontrollerfake.go index f7b968e78c..776e971643 100644 --- a/cns/fakes/requestcontrollerfake.go +++ b/cns/fakes/requestcontrollerfake.go @@ -115,7 +115,7 @@ func (rc *RequestControllerFake) Reconcile(removePendingReleaseIPs bool) error { } // update - rc.cnscli.PoolMonitor.Update(rc.NNC) + _ = rc.cnscli.PoolMonitor.Update(rc.NNC) return nil } diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 64fd1b3cf7..f5b657ae10 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -425,7 +425,7 @@ func createAndValidateNCRequest(t *testing.T, secondaryIPConfigs map[string]cns. if returnCode != 0 { t.Fatalf("Failed to createNetworkContainerRequest, req: %+v, err: %d", req, returnCode) } - svc.IPAMPoolMonitor.Update(&v1alpha.NodeNetworkConfig{ + _ = svc.IPAMPoolMonitor.Update(&v1alpha.NodeNetworkConfig{ Status: v1alpha.NodeNetworkConfigStatus{ Scaler: v1alpha.Scaler{ BatchSize: batchSize, @@ -598,7 +598,7 @@ func createNCReqInternal(t *testing.T, secondaryIPConfigs map[string]cns.Seconda if returnCode != 0 { t.Fatalf("Failed to createNetworkContainerRequest, req: %+v, err: %d", req, returnCode) } - svc.IPAMPoolMonitor.Update(&v1alpha.NodeNetworkConfig{ + _ = svc.IPAMPoolMonitor.Update(&v1alpha.NodeNetworkConfig{ Status: v1alpha.NodeNetworkConfigStatus{ Scaler: v1alpha.Scaler{ BatchSize: batchSize, diff --git a/cns/service/main.go b/cns/service/main.go index cecbd7fbad..23b6bc5444 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -865,19 +865,23 @@ func reconcileInitialCNSState(ctx context.Context, cli nodeNetworkConfigGetter, } // Convert to CreateNetworkContainerRequest - ncRequest, err := kubecontroller.CRDStatusToNCRequest(&nnc.Status) - if err != nil { - return errors.Wrap(err, "failed to convert NNC status to network container request") - } - // rebuild CNS state - podInfoByIP, err := podInfoByIPProvider.PodInfoByIP() - if err != nil { - return errors.Wrap(err, "provider failed to provide PodInfoByIP") - } + for i := range nnc.Status.NetworkContainers { + ncRequest, err := kubecontroller.CreateNCRequestFromDynamicNC(nnc.Status.NetworkContainers[i]) + if err != nil { + return errors.Wrap(err, "failed to convert NNC status to network container request") + } + // rebuild CNS state + podInfoByIP, err := podInfoByIPProvider.PodInfoByIP() + if err != nil { + return errors.Wrap(err, "provider failed to provide PodInfoByIP") + } - // Call cnsclient init cns passing those two things. - err = restserver.ResponseCodeToError(ncReconciler.ReconcileNCState(&ncRequest, podInfoByIP, nnc)) - return errors.Wrap(err, "failed to reconcile NC state") + // Call cnsclient init cns passing those two things. + if err := restserver.ResponseCodeToError(ncReconciler.ReconcileNCState(ncRequest, podInfoByIP, nnc)); err != nil { + return errors.Wrap(err, "failed to reconcile NC state") + } + } + return nil } // InitializeCRDState builds and starts the CRD controllers. @@ -1010,7 +1014,7 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn return errors.Wrapf(err, "failed to get node %s", nodeName) } - reconciler := kubecontroller.NewReconciler(nnccli, kubecontroller.SwiftNodeNetworkConfigListener(httpRestServiceImplementation), poolMonitor) + reconciler := kubecontroller.NewReconciler(httpRestServiceImplementation, nnccli, poolMonitor) // pass Node to the Reconciler for Controller xref if err := reconciler.SetupWithManager(manager, node); err != nil { return errors.Wrapf(err, "failed to setup reconciler with manager") diff --git a/cns/singletenantcontroller/conversion.go b/cns/singletenantcontroller/conversion.go index 8cf7dd2de0..afab6c034d 100644 --- a/cns/singletenantcontroller/conversion.go +++ b/cns/singletenantcontroller/conversion.go @@ -7,9 +7,6 @@ import ( "strings" "github.com/Azure/azure-container-networking/cns" - "github.com/Azure/azure-container-networking/cns/logger" - "github.com/Azure/azure-container-networking/cns/restserver" - cnstypes "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/pkg/errors" ) @@ -23,56 +20,9 @@ var ( ErrUnsupportedNCQuantity = errors.New("unsupported number of network containers") ) -type cnsClient interface { - CreateOrUpdateNetworkContainerInternal(*cns.CreateNetworkContainerRequest) cnstypes.ResponseCode -} - -var _ nodeNetworkConfigListener = (NodeNetworkConfigListenerFunc)(nil) //nolint:gocritic // clarity - -type NodeNetworkConfigListenerFunc func(*v1alpha.NodeNetworkConfig) error - -func (f NodeNetworkConfigListenerFunc) Update(nnc *v1alpha.NodeNetworkConfig) error { - return f(nnc) -} - -// SwiftNodeNetworkConfigListener return a function which satisfies the NodeNetworkConfigListener -// interface. It accepts a CreateOrUpdateNetworkContainerInternal implementation, and when Update -// is called, transforms the NNC in to an NC Request and calls the CNS Service implementation with -// that request. -func SwiftNodeNetworkConfigListener(cnscli cnsClient) NodeNetworkConfigListenerFunc { - return func(nnc *v1alpha.NodeNetworkConfig) error { - // Create NC request and hand it off to CNS - ncRequest, err := CRDStatusToNCRequest(&nnc.Status) - if err != nil { - return errors.Wrap(err, "failed to convert NNC status to network container request") - } - responseCode := cnscli.CreateOrUpdateNetworkContainerInternal(&ncRequest) - err = restserver.ResponseCodeToError(responseCode) - if err != nil { - logger.Errorf("[cns-rc] Error creating or updating NC in reconcile: %v", err) - return errors.Wrap(err, "failed to create or update network container") - } - - // record assigned IPs metric - allocatedIPs.Set(float64(len(nnc.Status.NetworkContainers[0].IPAssignments))) - return nil - } -} - -// CRDStatusToNCRequest translates a crd status to createnetworkcontainer request -func CRDStatusToNCRequest(status *v1alpha.NodeNetworkConfigStatus) (cns.CreateNetworkContainerRequest, error) { - // if NNC has no NC, return an empty request - if len(status.NetworkContainers) == 0 { - return cns.CreateNetworkContainerRequest{}, nil - } - - // only support a single NC per node, error on more - if len(status.NetworkContainers) > 1 { - return cns.CreateNetworkContainerRequest{}, errors.Wrapf(ErrUnsupportedNCQuantity, "count: %d", len(status.NetworkContainers)) - } - - nc := status.NetworkContainers[0] - +// CreateNCRequestFromDynamicNC generates a CreateNetworkContainerRequest from a dynamic NetworkContainer. +//nolint:gocritic //ignore hugeparam +func CreateNCRequestFromDynamicNC(nc v1alpha.NetworkContainer) (*cns.CreateNetworkContainerRequest, error) { primaryIP := nc.PrimaryIP // if the PrimaryIP is not a CIDR, append a /32 if !strings.Contains(primaryIP, "/") { @@ -81,35 +31,75 @@ func CRDStatusToNCRequest(status *v1alpha.NodeNetworkConfigStatus) (cns.CreateNe primaryPrefix, err := netip.ParsePrefix(primaryIP) if err != nil { - return cns.CreateNetworkContainerRequest{}, errors.Wrapf(err, "IP: %s", primaryIP) + return nil, errors.Wrapf(err, "IP: %s", primaryIP) } - secondaryPrefix, err := netip.ParsePrefix(nc.SubnetAddressSpace) + subnetPrefix, err := netip.ParsePrefix(nc.SubnetAddressSpace) if err != nil { - return cns.CreateNetworkContainerRequest{}, errors.Wrapf(err, "invalid SubnetAddressSpace %s", nc.SubnetAddressSpace) + return nil, errors.Wrapf(err, "invalid SubnetAddressSpace %s", nc.SubnetAddressSpace) } subnet := cns.IPSubnet{ IPAddress: primaryPrefix.Addr().String(), - PrefixLength: uint8(secondaryPrefix.Bits()), + PrefixLength: uint8(subnetPrefix.Bits()), } secondaryIPConfigs := map[string]cns.SecondaryIPConfig{} for _, ipAssignment := range nc.IPAssignments { secondaryIP := net.ParseIP(ipAssignment.IP) if secondaryIP == nil { - return cns.CreateNetworkContainerRequest{}, errors.Wrapf(ErrInvalidSecondaryIP, "IP: %s", ipAssignment.IP) + return nil, errors.Wrapf(ErrInvalidSecondaryIP, "IP: %s", ipAssignment.IP) } secondaryIPConfigs[ipAssignment.Name] = cns.SecondaryIPConfig{ IPAddress: secondaryIP.String(), NCVersion: int(nc.Version), } } - return cns.CreateNetworkContainerRequest{ + return &cns.CreateNetworkContainerRequest{ + SecondaryIPConfigs: secondaryIPConfigs, + NetworkContainerid: nc.ID, + NetworkContainerType: cns.Docker, + Version: strconv.FormatInt(nc.Version, 10), //nolint:gomnd // it's decimal + IPConfiguration: cns.IPConfiguration{ + IPSubnet: subnet, + GatewayIPAddress: nc.DefaultGateway, + }, + }, nil +} + +// CreateNCRequestFromStaticNC generates a CreateNetworkContainerRequest from a static NetworkContainer. +//nolint:gocritic //ignore hugeparam +func CreateNCRequestFromStaticNC(nc v1alpha.NetworkContainer) (*cns.CreateNetworkContainerRequest, error) { + primaryPrefix, err := netip.ParsePrefix(nc.PrimaryIP) + if err != nil { + return nil, errors.Wrapf(err, "IP: %s", nc.PrimaryIP) + } + + subnetPrefix, err := netip.ParsePrefix(nc.SubnetAddressSpace) + if err != nil { + return nil, errors.Wrapf(err, "invalid SubnetAddressSpace %s", nc.SubnetAddressSpace) + } + subnet := cns.IPSubnet{ + IPAddress: primaryPrefix.Addr().String(), + PrefixLength: uint8(subnetPrefix.Bits()), + } + + secondaryIPConfigs := map[string]cns.SecondaryIPConfig{} + + // iterate through all IP addresses in the subnet described by primaryPrefix and + // add them to the request as secondary IPConfigs. + zeroAddr := primaryPrefix.Masked().Addr() // the masked address is the 0th IP in the subnet + for addr := zeroAddr.Next(); primaryPrefix.Contains(addr); addr = addr.Next() { + secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{ + IPAddress: addr.String(), + NCVersion: int(nc.Version), + } + } + return &cns.CreateNetworkContainerRequest{ SecondaryIPConfigs: secondaryIPConfigs, NetworkContainerid: nc.ID, NetworkContainerType: cns.Docker, - Version: strconv.FormatInt(nc.Version, 10), + Version: strconv.FormatInt(nc.Version, 10), //nolint:gomnd // it's decimal IPConfiguration: cns.IPConfiguration{ IPSubnet: subnet, GatewayIPAddress: nc.DefaultGateway, diff --git a/cns/singletenantcontroller/conversion_test.go b/cns/singletenantcontroller/conversion_test.go index 4dd4ea4651..3df4e0fc98 100644 --- a/cns/singletenantcontroller/conversion_test.go +++ b/cns/singletenantcontroller/conversion_test.go @@ -16,6 +16,7 @@ const ( ipMalformed = "10.0.0.0.0" ncID = "160005ba-cd02-11ea-87d0-0242ac130003" primaryIP = "10.0.0.1" + overlayPrimaryIP = "10.0.0.1/30" subnetAddressSpace = "10.0.0.0/24" subnetName = "subnet1" subnetPrefixLen = 24 @@ -30,29 +31,30 @@ var invalidStatusMultiNC = v1alpha.NodeNetworkConfigStatus{ }, } -var validStatus = v1alpha.NodeNetworkConfigStatus{ - NetworkContainers: []v1alpha.NetworkContainer{ +var validSwiftNC = v1alpha.NetworkContainer{ + ID: ncID, + AssignmentMode: v1alpha.Dynamic, + Type: v1alpha.VNET, + PrimaryIP: primaryIP, + IPAssignments: []v1alpha.IPAssignment{ { - PrimaryIP: primaryIP, - ID: ncID, - IPAssignments: []v1alpha.IPAssignment{ - { - Name: uuid, - IP: testSecIP, - }, - }, - SubnetName: subnetName, - DefaultGateway: defaultGateway, - SubnetAddressSpace: subnetAddressSpace, - Version: version, + Name: uuid, + IP: testSecIP, }, }, - Scaler: v1alpha.Scaler{ - BatchSize: 1, + SubnetName: subnetName, + DefaultGateway: defaultGateway, + SubnetAddressSpace: subnetAddressSpace, + Version: version, +} + +var validSwiftStatus = v1alpha.NodeNetworkConfigStatus{ + NetworkContainers: []v1alpha.NetworkContainer{ + validSwiftNC, }, } -var validRequest = cns.CreateNetworkContainerRequest{ +var validSwiftRequest = &cns.CreateNetworkContainerRequest{ Version: strconv.FormatInt(version, 10), IPConfiguration: cns.IPConfiguration{ GatewayIPAddress: defaultGateway, @@ -71,129 +73,221 @@ var validRequest = cns.CreateNetworkContainerRequest{ }, } -func TestConvertNNCStatusToNCRequest(t *testing.T) { +var validOverlayNC = v1alpha.NetworkContainer{ + ID: ncID, + AssignmentMode: v1alpha.Static, + Type: v1alpha.Overlay, + PrimaryIP: overlayPrimaryIP, + SubnetName: subnetName, + SubnetAddressSpace: subnetAddressSpace, + Version: version, +} + +var validOverlayRequest = &cns.CreateNetworkContainerRequest{ + Version: strconv.FormatInt(version, 10), + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + PrefixLength: uint8(subnetPrefixLen), + IPAddress: primaryIP, + }, + }, + NetworkContainerid: ncID, + NetworkContainerType: cns.Docker, + SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{ + "10.0.0.1": { + IPAddress: "10.0.0.1", + NCVersion: version, + }, + "10.0.0.2": { + IPAddress: "10.0.0.2", + NCVersion: version, + }, + "10.0.0.3": { + IPAddress: "10.0.0.3", + NCVersion: version, + }, + }, +} + +func TestCreateNCRequestFromDynamicNC(t *testing.T) { tests := []struct { name string - input v1alpha.NodeNetworkConfigStatus - want cns.CreateNetworkContainerRequest + input v1alpha.NetworkContainer + want *cns.CreateNetworkContainerRequest wantErr bool }{ { - name: "valid", - input: validStatus, + name: "valid swift", + input: validSwiftNC, wantErr: false, - want: validRequest, + want: validSwiftRequest, }, { - name: "no nc", - input: v1alpha.NodeNetworkConfigStatus{}, - wantErr: false, - want: cns.CreateNetworkContainerRequest{}, + name: "malformed primary IP", + input: v1alpha.NetworkContainer{ + PrimaryIP: ipMalformed, + ID: ncID, + IPAssignments: []v1alpha.IPAssignment{ + { + Name: uuid, + IP: testSecIP, + }, + }, + SubnetAddressSpace: subnetAddressSpace, + }, + + wantErr: true, }, { - name: ">1 nc", - input: invalidStatusMultiNC, + name: "malformed IP assignment", + input: v1alpha.NetworkContainer{ + PrimaryIP: primaryIP, + ID: ncID, + IPAssignments: []v1alpha.IPAssignment{ + { + Name: uuid, + IP: ipMalformed, + }, + }, + SubnetAddressSpace: subnetAddressSpace, + }, wantErr: true, }, { - name: "malformed primary IP", - input: v1alpha.NodeNetworkConfigStatus{ - NetworkContainers: []v1alpha.NetworkContainer{ + name: "IP is CIDR", + input: v1alpha.NetworkContainer{ + PrimaryIP: ipIsCIDR, + ID: ncID, + IPAssignments: []v1alpha.IPAssignment{ { - PrimaryIP: ipMalformed, - ID: ncID, - IPAssignments: []v1alpha.IPAssignment{ - { - Name: uuid, - IP: testSecIP, - }, - }, - SubnetAddressSpace: subnetAddressSpace, + Name: uuid, + IP: testSecIP, }, }, + SubnetName: subnetName, + DefaultGateway: defaultGateway, + SubnetAddressSpace: subnetAddressSpace, + Version: version, + }, + wantErr: false, + want: validSwiftRequest, + }, + { + name: "IP assignment is CIDR", + input: v1alpha.NetworkContainer{ + PrimaryIP: primaryIP, + ID: ncID, + IPAssignments: []v1alpha.IPAssignment{ + { + Name: uuid, + IP: ipIsCIDR, + }, + }, + SubnetAddressSpace: subnetAddressSpace, }, wantErr: true, }, { - name: "malformed IP assignment", - input: v1alpha.NodeNetworkConfigStatus{ - NetworkContainers: []v1alpha.NetworkContainer{ + name: "address space is not CIDR", + input: v1alpha.NetworkContainer{ + PrimaryIP: primaryIP, + ID: ncID, + IPAssignments: []v1alpha.IPAssignment{ { - PrimaryIP: primaryIP, - ID: ncID, - IPAssignments: []v1alpha.IPAssignment{ - { - Name: uuid, - IP: ipMalformed, - }, - }, - SubnetAddressSpace: subnetAddressSpace, + Name: uuid, + IP: testSecIP, }, }, + SubnetAddressSpace: "10.0.0.0", // not a cidr range }, wantErr: true, }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + got, err := CreateNCRequestFromDynamicNC(tt.input) + if tt.wantErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.EqualValues(t, tt.want, got) + }) + } +} + +func TestCreateNCRequestFromStaticNC(t *testing.T) { + tests := []struct { + name string + input v1alpha.NetworkContainer + want *cns.CreateNetworkContainerRequest + wantErr bool + }{ + { + name: "valid overlay", + input: validOverlayNC, + wantErr: false, + want: validOverlayRequest, + }, { - name: "IP is CIDR", - input: v1alpha.NodeNetworkConfigStatus{ - NetworkContainers: []v1alpha.NetworkContainer{ + name: "malformed primary IP", + input: v1alpha.NetworkContainer{ + PrimaryIP: ipMalformed, + ID: ncID, + IPAssignments: []v1alpha.IPAssignment{ { - PrimaryIP: ipIsCIDR, - ID: ncID, - IPAssignments: []v1alpha.IPAssignment{ - { - Name: uuid, - IP: testSecIP, - }, - }, - SubnetName: subnetName, - DefaultGateway: defaultGateway, - SubnetAddressSpace: subnetAddressSpace, - Version: version, + Name: uuid, + IP: testSecIP, }, }, - Scaler: v1alpha.Scaler{ - BatchSize: 1, + SubnetAddressSpace: subnetAddressSpace, + }, + + wantErr: true, + }, + { + name: "malformed IP assignment", + input: v1alpha.NetworkContainer{ + PrimaryIP: primaryIP, + ID: ncID, + IPAssignments: []v1alpha.IPAssignment{ + { + Name: uuid, + IP: ipMalformed, + }, }, + SubnetAddressSpace: subnetAddressSpace, }, - wantErr: false, - want: validRequest, + wantErr: true, }, { name: "IP assignment is CIDR", - input: v1alpha.NodeNetworkConfigStatus{ - NetworkContainers: []v1alpha.NetworkContainer{ + input: v1alpha.NetworkContainer{ + PrimaryIP: primaryIP, + ID: ncID, + IPAssignments: []v1alpha.IPAssignment{ { - PrimaryIP: primaryIP, - ID: ncID, - IPAssignments: []v1alpha.IPAssignment{ - { - Name: uuid, - IP: ipIsCIDR, - }, - }, - SubnetAddressSpace: subnetAddressSpace, + Name: uuid, + IP: ipIsCIDR, }, }, + SubnetAddressSpace: subnetAddressSpace, }, wantErr: true, }, { name: "address space is not CIDR", - input: v1alpha.NodeNetworkConfigStatus{ - NetworkContainers: []v1alpha.NetworkContainer{ + input: v1alpha.NetworkContainer{ + PrimaryIP: primaryIP, + ID: ncID, + IPAssignments: []v1alpha.IPAssignment{ { - PrimaryIP: primaryIP, - ID: ncID, - IPAssignments: []v1alpha.IPAssignment{ - { - Name: uuid, - IP: testSecIP, - }, - }, - SubnetAddressSpace: "10.0.0.0", // not a cidr range + Name: uuid, + IP: testSecIP, }, }, + SubnetAddressSpace: "10.0.0.0", // not a cidr range }, wantErr: true, }, @@ -201,7 +295,7 @@ func TestConvertNNCStatusToNCRequest(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - got, err := CRDStatusToNCRequest(&tt.input) + got, err := CreateNCRequestFromStaticNC(tt.input) if tt.wantErr { assert.Error(t, err) return diff --git a/cns/singletenantcontroller/reconciler.go b/cns/singletenantcontroller/reconciler.go index e0b436911b..7ebbbdb308 100644 --- a/cns/singletenantcontroller/reconciler.go +++ b/cns/singletenantcontroller/reconciler.go @@ -4,7 +4,10 @@ import ( "context" "sync" + "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/logger" + "github.com/Azure/azure-container-networking/cns/restserver" + cnstypes "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/pkg/errors" v1 "k8s.io/api/core/v1" @@ -18,6 +21,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" ) +type cnsClient interface { + CreateOrUpdateNetworkContainerInternal(*cns.CreateNetworkContainerRequest) cnstypes.ResponseCode +} + type nodeNetworkConfigListener interface { Update(*v1alpha.NodeNetworkConfig) error } @@ -28,26 +35,29 @@ type nncGetter interface { // Reconciler watches for CRD status changes type Reconciler struct { - nncListeners []nodeNetworkConfigListener - nnccli nncGetter - once sync.Once - started chan interface{} + cnscli cnsClient + ipampoolmonitorcli nodeNetworkConfigListener + nnccli nncGetter + once sync.Once + started chan interface{} } // NewReconciler creates a NodeNetworkConfig Reconciler which will get updates from the Kubernetes // apiserver for NNC events. // Provided nncListeners are passed the NNC after the Reconcile preprocesses it. Note: order matters! The // passed Listeners are notified in the order provided. -func NewReconciler(nnccli nncGetter, nncListeners ...nodeNetworkConfigListener) *Reconciler { +func NewReconciler(cnscli cnsClient, nnccli nncGetter, ipampoolmonitorcli nodeNetworkConfigListener) *Reconciler { return &Reconciler{ - nncListeners: nncListeners, - nnccli: nnccli, - started: make(chan interface{}), + cnscli: cnscli, + ipampoolmonitorcli: ipampoolmonitorcli, + nnccli: nnccli, + started: make(chan interface{}), } } // Reconcile is called on CRD status changes func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { + listenersToNotify := []nodeNetworkConfigListener{} nnc, err := r.nnccli.Get(ctx, req.NamespacedName) if err != nil { if apierrors.IsNotFound(err) { @@ -60,15 +70,44 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco logger.Printf("[cns-rc] CRD Spec: %v", nnc.Spec) - // If there are no network containers, don't continue to updating Listeners + // if there are no network containers, don't continue to updating Listeners if len(nnc.Status.NetworkContainers) == 0 { logger.Errorf("[cns-rc] Empty NetworkContainers") return reconcile.Result{}, nil } - // push the NNC to the registered NNC Sinks - for i := range r.nncListeners { - if err := r.nncListeners[i].Update(nnc); err != nil { + ipAssignments := 0 + // for each NC, parse it in to a CreateNCRequest and forward it to the appropriate Listener + for i := range nnc.Status.NetworkContainers { + var req *cns.CreateNetworkContainerRequest + var err error + switch nnc.Status.NetworkContainers[i].AssignmentMode { + case v1alpha.Dynamic: + req, err = CreateNCRequestFromDynamicNC(nnc.Status.NetworkContainers[i]) + // in dynamic, we will also push this NNC to the IPAM Pool Monitor when we're done. + listenersToNotify = append(listenersToNotify, r.ipampoolmonitorcli) + case v1alpha.Static: + req, err = CreateNCRequestFromStaticNC(nnc.Status.NetworkContainers[i]) + default: + // unrecognized mode, fail out + err = errors.Errorf("unknown NetworkContainer AssignmentMode %s", string(nnc.Status.NetworkContainers[i].AssignmentMode)) + } + if err != nil { + return reconcile.Result{}, errors.Wrap(err, "failed to generate CreateNCRequest from NC") + } + responseCode := r.cnscli.CreateOrUpdateNetworkContainerInternal(req) + if err := restserver.ResponseCodeToError(responseCode); err != nil { + logger.Errorf("[cns-rc] Error creating or updating NC in reconcile: %v", err) + return reconcile.Result{}, errors.Wrap(err, "failed to create or update network container") + } + ipAssignments += len(req.SecondaryIPConfigs) + } + // record assigned IPs metric + allocatedIPs.Set(float64(ipAssignments)) + + // push the NNC to the registered NNC listeners. + for _, l := range listenersToNotify { + if err := l.Update(nnc); err != nil { return reconcile.Result{}, errors.Wrap(err, "nnc listener return error during update") } } diff --git a/cns/singletenantcontroller/reconciler_test.go b/cns/singletenantcontroller/reconciler_test.go index 39c9cd2e4e..bb5619417f 100644 --- a/cns/singletenantcontroller/reconciler_test.go +++ b/cns/singletenantcontroller/reconciler_test.go @@ -28,7 +28,6 @@ type mockCNSClient struct { update func(*v1alpha.NodeNetworkConfig) error } -//nolint:gocritic // ignore hugeParam pls func (m *mockCNSClient) CreateOrUpdateNetworkContainerInternal(req *cns.CreateNetworkContainerRequest) cnstypes.ResponseCode { m.state.req = req return m.createOrUpdateNC(req) @@ -101,7 +100,7 @@ func TestReconcile(t *testing.T) { ncGetter: mockNCGetter{ get: func(context.Context, types.NamespacedName) (*v1alpha.NodeNetworkConfig, error) { return &v1alpha.NodeNetworkConfig{ - Status: validStatus, + Status: validSwiftStatus, }, nil }, }, @@ -112,7 +111,7 @@ func TestReconcile(t *testing.T) { }, wantErr: true, wantCNSClientState: cnsClientState{ - req: &validRequest, + req: validSwiftRequest, }, }, { @@ -120,7 +119,7 @@ func TestReconcile(t *testing.T) { ncGetter: mockNCGetter{ get: func(context.Context, types.NamespacedName) (*v1alpha.NodeNetworkConfig, error) { return &v1alpha.NodeNetworkConfig{ - Status: validStatus, + Status: validSwiftStatus, Spec: v1alpha.NodeNetworkConfigSpec{ RequestedIPCount: 1, }, @@ -137,9 +136,9 @@ func TestReconcile(t *testing.T) { }, wantErr: false, wantCNSClientState: cnsClientState{ - req: &validRequest, + req: validSwiftRequest, nnc: &v1alpha.NodeNetworkConfig{ - Status: validStatus, + Status: validSwiftStatus, Spec: v1alpha.NodeNetworkConfigSpec{ RequestedIPCount: 1, }, @@ -150,13 +149,13 @@ func TestReconcile(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - r := NewReconciler(&tt.ncGetter, SwiftNodeNetworkConfigListener(&tt.cnsClient), &tt.cnsClient) + r := NewReconciler(&tt.cnsClient, &tt.ncGetter, &tt.cnsClient) got, err := r.Reconcile(context.Background(), tt.in) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) assert.Equal(t, tt.want, got) assert.Equal(t, tt.wantCNSClientState, tt.cnsClient.state) })