Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cns/common/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/Azure/azure-container-networking/server/tls"
"github.com/Azure/azure-container-networking/store"
"go.uber.org/zap"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// Service implements behavior common to all services.
Expand All @@ -20,6 +21,7 @@ type Service struct {
Options map[string]interface{}
ErrChan chan<- error
Store store.KeyValueStore
Client client.Client
ChannelMode string
Logger *zap.Logger
}
Expand All @@ -42,6 +44,7 @@ type ServiceConfig struct {
Store store.KeyValueStore
Server server
ChannelMode string
Client client.Client
TLSSettings tls.TlsSettings
Logger *zap.Logger
}
Expand Down Expand Up @@ -82,6 +85,7 @@ func (service *Service) Initialize(config *ServiceConfig) error {
service.ErrChan = config.ErrChan
service.Store = config.Store
service.Version = config.Version
service.Client = config.Client
service.ChannelMode = config.ChannelMode
service.Logger = config.Logger

Expand Down
21 changes: 21 additions & 0 deletions cns/restserver/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -1337,3 +1337,24 @@ func (service *HTTPRestService) nmAgentNCListHandler(w http.ResponseWriter, r *h
serviceErr := common.Encode(w, &NCListResponse)
logger.Response(service.Name, NCListResponse, resp.ReturnCode, serviceErr)
}

// ibDevicesHandler handles IB device operations
func (service *HTTPRestService) ibDevicesHandler(w http.ResponseWriter, r *http.Request) {
opName := "ibDevicesHandler"
logger.Printf("[%s] Received request with HTTP method %s", opName, r.Method)

service.Lock()
defer service.Unlock()

switch r.Method {
case http.MethodPost:
service.assignIBDevicesToPod(w, r)
case http.MethodGet:
// TODO: Implement GET
http.Error(w, "GET method not yet implemented", http.StatusNotImplemented)
default:
msg := fmt.Sprintf("[%s] Method %s not supported", opName, r.Method)
http.Error(w, msg, http.StatusMethodNotAllowed)
logger.Response(service.Name, nil, types.InvalidParameter, errors.New(msg))
}
}
33 changes: 33 additions & 0 deletions cns/restserver/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"encoding/xml"
"fmt"
"io"
"net"
"net/http"
"net/http/httptest"
"net/url"
Expand Down Expand Up @@ -1916,6 +1917,38 @@
assert.Equal(t, types.UnexpectedError, vmIDResp.Response.ReturnCode)
}

func TestIBDevices(t *testing.T) {
var (
err error
req *http.Request
ibDevicesReq cns.AssignIBDevicesToPodRequest
body bytes.Buffer
)

ibDevicesReq.PodName = "testpod"
ibDevicesReq.PodNamespace = "testpodnamespace"
ibDevicesReq.IBMACAddresses = []net.HardwareAddr{
{0x00, 0x1A, 0x2B, 0x3C, 0x4D, 0x5E},
{0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x01},
}

json.NewEncoder(&body).Encode(ibDevicesReq)

Check failure on line 1935 in cns/restserver/api_test.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

Error return value of `(*encoding/json.Encoder).Encode` is not checked (errcheck)

Check failure on line 1935 in cns/restserver/api_test.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

Error return value of `(*encoding/json.Encoder).Encode` is not checked (errcheck)

Check failure on line 1935 in cns/restserver/api_test.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

Error return value of `(*encoding/json.Encoder).Encode` is not checked (errcheck)

Check failure on line 1935 in cns/restserver/api_test.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

Error return value of `(*encoding/json.Encoder).Encode` is not checked (errcheck)
req, err = http.NewRequest(http.MethodPost, cns.IBDevicesPath, &body)
if err != nil {
t.Fatal(err)
}

w := httptest.NewRecorder()
mux.ServeHTTP(w, req)
var ibDevicesResponse cns.AssignIBDevicesToPodResponse

if err = decodeResponse(w, &ibDevicesResponse); err != nil {
t.Errorf("AssignIBDevicesToPod failed with response %+v and error %v", ibDevicesResponse, err)
}

fmt.Printf("Raw response: %+v", w.Body)
}

// IGNORE TEST AS IT IS FAILING. TODO:- Fix it https://msazure.visualstudio.com/One/_workitems/edit/7720083
// // Tests CreateNetwork functionality.

Expand Down
244 changes: 244 additions & 0 deletions cns/restserver/ibdevices.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,244 @@
package restserver

import (
"context"
"fmt"
"net"
"net/http"
"sort"

"github.com/pkg/errors"

Check failure on line 10 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

File is not properly formatted (gci)

Check failure on line 10 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

File is not properly formatted (gci)

Check failure on line 10 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

File is not properly formatted (gci)

Check failure on line 10 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

File is not properly formatted (gci)

"github.com/Azure/azure-container-networking/cns"
"github.com/Azure/azure-container-networking/cns/logger"
"github.com/Azure/azure-container-networking/cns/types"
"github.com/Azure/azure-container-networking/common"
"github.com/Azure/azure-container-networking/crd/multitenancy"
"github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1"
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8stypes "k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
)

const (
// NUMALabel is the label key used to indicate if a pod requires NUMA-aware IB device assignment
NUMALabel = "numa-aware-ib-device-assignment"
PodNetworkInstance = "pod-network-instance"
PNILabel = "kubernetes.azure.com/pod-network-instance"
fieldOwner = "requestcontroller"
)

// assignIBDevicesToPod handles POST requests to assign IB devices to a pod
func (service *HTTPRestService) assignIBDevicesToPod(w http.ResponseWriter, r *http.Request) {
opName := "assignIBDevicesToPod"
var req cns.AssignIBDevicesToPodRequest
var response cns.AssignIBDevicesToPodResponse
ctx := context.Background()
pod := &v1.Pod{}

// Decode the request
err := common.Decode(w, r, &req)
logger.Request(service.Name, &req, err)
if err != nil {
response.Message = fmt.Sprintf("Failed to decode request: %v", err)
respond(opName, w, http.StatusBadRequest, types.InvalidRequest, response)
return
}

// Validate the request
if err := validateAssignIBDevicesRequest(req); err != nil {
response.Message = fmt.Sprintf("Invalid request: %v", err)
respond(opName, w, http.StatusBadRequest, types.InvalidRequest, response)
return
}

// Format the pod name and namespace into a k8s 'namespaced name'
podNamespacedName := k8stypes.NamespacedName{
Namespace: req.PodNamespace,
Name: req.PodName,
}

// Get the pod
if err := service.Client.Get(ctx, podNamespacedName, pod); err != nil {

Check failure on line 65 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

Non-inherited new context, use function like `context.WithXXX` or `r.Context` instead (contextcheck)

Check failure on line 65 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

Non-inherited new context, use function like `context.WithXXX` or `r.Context` instead (contextcheck)

Check failure on line 65 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

Non-inherited new context, use function like `context.WithXXX` or `r.Context` instead (contextcheck)

Check failure on line 65 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

Non-inherited new context, use function like `context.WithXXX` or `r.Context` instead (contextcheck)
response.Message = fmt.Sprintf("Failed to get pod %s/%s: %v", req.PodNamespace, req.PodName, err)
respond(opName, w, http.StatusInternalServerError, types.UnexpectedError, response)
return
}

// Check that the pod has the NUMA label
if !podHasNUMALabel(pod) {
response.Message = fmt.Sprintf("Pod %s/%s does not have the required NUMA label %s",
req.PodNamespace, req.PodName, NUMALabel)
respond(opName, w, http.StatusBadRequest, types.InvalidRequest, response)
return
}

// Check if the requested IB devices are unprogrammed
for _, ibMAC := range req.IBMACAddresses {
if !IBDeviceIsUnprogrammed(ibMAC) {
response.Message = fmt.Sprintf("IB device with MAC address %s is not unprogrammed", ibMAC)
respond(opName, w, http.StatusBadRequest, types.AddressUnavailable, response)
return
}
}

// Create MTPNC with IB devices in spec
if err = service.createMTPNC(ctx, pod, req.IBMACAddresses); err != nil {

Check failure on line 89 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

Non-inherited new context, use function like `context.WithXXX` or `r.Context` instead (contextcheck)

Check failure on line 89 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

Non-inherited new context, use function like `context.WithXXX` or `r.Context` instead (contextcheck)

Check failure on line 89 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

Non-inherited new context, use function like `context.WithXXX` or `r.Context` instead (contextcheck)

Check failure on line 89 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

Non-inherited new context, use function like `context.WithXXX` or `r.Context` instead (contextcheck)
response.Message = fmt.Sprintf("Failed to create MTPNC for pod %s/%s: %v", req.PodNamespace, req.PodName, err)
respond(opName, w, http.StatusInternalServerError, types.UnexpectedError, response)
return
}

// Report back a successful assignment
response.Message = fmt.Sprintf("Successfully assigned %d IB devices to pod %s/%s",
len(req.IBMACAddresses), req.PodNamespace, req.PodName)
respond(opName, w, http.StatusOK, types.Success, response)
}

func validateAssignIBDevicesRequest(req cns.AssignIBDevicesToPodRequest) error {
if req.PodName == "" || req.PodNamespace == "" {
return fmt.Errorf("pod name and namespace are required")

Check failure on line 103 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf(\"pod name and namespace are required\")" (err113)

Check failure on line 103 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf(\"pod name and namespace are required\")" (err113)

Check failure on line 103 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf(\"pod name and namespace are required\")" (err113)

Check failure on line 103 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf(\"pod name and namespace are required\")" (err113)
}
if len(req.IBMACAddresses) == 0 {
return fmt.Errorf("at least one IB MAC address is required")

Check failure on line 106 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf(\"at least one IB MAC address is required\")" (err113)

Check failure on line 106 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf(\"at least one IB MAC address is required\")" (err113)

Check failure on line 106 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf(\"at least one IB MAC address is required\")" (err113)

Check failure on line 106 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf(\"at least one IB MAC address is required\")" (err113)
}
// Validate MAC address format - since they're already net.HardwareAddr, they should be valid
for _, hwAddr := range req.IBMACAddresses {
if len(hwAddr) == 0 {
return fmt.Errorf("invalid empty MAC address")

Check failure on line 111 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf(\"invalid empty MAC address\")" (err113)

Check failure on line 111 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf(\"invalid empty MAC address\")" (err113)

Check failure on line 111 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf(\"invalid empty MAC address\")" (err113)

Check failure on line 111 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf(\"invalid empty MAC address\")" (err113)
}
}
return nil
}

func respond(opName string, w http.ResponseWriter, httpStatusCode int, cnsCode types.ResponseCode, response interface{}) {
w.WriteHeader(httpStatusCode)
_ = common.Encode(w, &response)
logger.Response(opName, response, cnsCode, errors.New(fmt.Sprintf("HTTP: %v CNSCode:%v Response: %v", httpStatusCode, cnsCode, response)))

Check failure on line 120 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

errorf: should replace errors.New(fmt.Sprintf(...)) with fmt.Errorf(...) (revive)

Check failure on line 120 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

errorf: should replace errors.New(fmt.Sprintf(...)) with fmt.Errorf(...) (revive)

Check failure on line 120 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

errorf: should replace errors.New(fmt.Sprintf(...)) with fmt.Errorf(...) (revive)

Check failure on line 120 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

errorf: should replace errors.New(fmt.Sprintf(...)) with fmt.Errorf(...) (revive)
}

func podHasNUMALabel(pod *v1.Pod) bool {
if pod == nil || pod.Labels == nil {
return false
}
_, hasLabel := pod.Labels[NUMALabel]
return hasLabel
}

// TODO: Finish this
func IBDeviceIsUnprogrammed(ibMAC net.HardwareAddr) bool {

Check failure on line 132 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

unused-parameter: parameter 'ibMAC' seems to be unused, consider removing or renaming it as _ (revive)

Check failure on line 132 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

unused-parameter: parameter 'ibMAC' seems to be unused, consider removing or renaming it as _ (revive)

Check failure on line 132 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

unused-parameter: parameter 'ibMAC' seems to be unused, consider removing or renaming it as _ (revive)

Check failure on line 132 in cns/restserver/ibdevices.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

unused-parameter: parameter 'ibMAC' seems to be unused, consider removing or renaming it as _ (revive)
// Check if the IB device is available (i.e., not assigned to any pod)
// This is a placeholder implementation and should be replaced with actual logic
return true
}

func (service *HTTPRestService) createMTPNC(ctx context.Context, pod *v1.Pod, ibMACs []net.HardwareAddr) error {
// create the MTPNC for the pod
mtpnc := &v1alpha1.MultitenantPodNetworkConfig{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is IBMACs being populated? and we should assign Unprogrammed status here if ib mac not in-use

ObjectMeta: metav1.ObjectMeta{
Name: pod.Name,
Namespace: pod.Namespace,
},
Spec: v1alpha1.MultitenantPodNetworkConfigSpec{
PodNetworkInstance: pod.Labels[PNILabel],
IBMACAddresses: convertMACsToStrings(ibMACs),
},
}

if err := controllerutil.SetControllerReference(pod, mtpnc, multitenancy.Scheme); err != nil {
return errors.Wrap(err, "unable to set controller reference for mtpnc")
}

if createErr := service.Client.Create(ctx, mtpnc); createErr != nil {
// return any creation error except IsAlreadyExists
if !apierrors.IsAlreadyExists(createErr) {
return errors.Wrap(createErr, "error creating mtpnc")
}

existingMTPNC := &v1alpha1.MultitenantPodNetworkConfig{}
if getErr := service.Client.Get(ctx, k8stypes.NamespacedName{Name: mtpnc.Name, Namespace: mtpnc.Namespace}, existingMTPNC); getErr != nil {
return errors.Wrap(getErr, "mtpnc already exists, but got error while reading it from apiserver")
}

// If the ownership or spec is wrong, try to patch it. We can't really support updates because once the MTPNC has an IP, we don't
// take it away, but it's possible that the customer created a MTPNC manually and we don't want to get stuck if they did, so
// we'll just make a best effort to keep the MTPNC up-to-date with the Pod.
if patch, patchRequired := determineMTPNCUpdate(existingMTPNC, mtpnc); patchRequired {
if patchErr := service.Client.Patch(ctx, patch, client.Apply, client.ForceOwnership, client.FieldOwner(fieldOwner)); patchErr != nil {
return errors.Wrap(patchErr, "mtpnc requires an update but got error while patching")
}
service.Logger.Info(fmt.Sprintf("Patched existing MTPNC %s/%s to match desired state", mtpnc.Namespace, mtpnc.Name))
}
}
return nil
}

func convertMACsToStrings(macAddrs []net.HardwareAddr) []string {
macStrs := make([]string, 0, len(macAddrs))
for _, hwAddr := range macAddrs {
macStrs = append(macStrs, hwAddr.String())
}
return macStrs
}

// determineMTPNCUpdate compares the ownership references and specs of the two MTPNC objects and returns a MTPNC for patching to the
// desired state and true. If no update is required, this will return nil and false
func determineMTPNCUpdate(existing, desired *v1alpha1.MultitenantPodNetworkConfig) (*v1alpha1.MultitenantPodNetworkConfig, bool) {
patchRequired := false
patchSkel := &v1alpha1.MultitenantPodNetworkConfig{
ObjectMeta: metav1.ObjectMeta{
Name: existing.Name,
Namespace: existing.Namespace,
},
}

if !ownerReferencesEqual(existing.OwnerReferences, desired.OwnerReferences) {
patchRequired = true
patchSkel.OwnerReferences = desired.OwnerReferences
}

if patchRequired {
return patchSkel, true
}

return nil, false
}

func ownerReferencesEqual(o1, o2 []metav1.OwnerReference) bool {
if len(o1) != len(o2) {
return false
}

// sort the slices by UID
sort.Slice(o1, func(i, j int) bool {
return o1[i].UID < o1[j].UID
})
sort.Slice(o2, func(i, j int) bool {
return o2[i].UID < o2[j].UID
})

// compare each owner ref
equal := true
for i := range o1 {
equal = equal &&
o1[i].Kind == o2[i].Kind &&
o1[i].Name == o2[i].Name &&
o1[i].UID == o2[i].UID &&
o1[i].APIVersion == o2[i].APIVersion &&
boolPtrsEqual(o1[i].Controller, o2[i].Controller) &&
boolPtrsEqual(o1[i].BlockOwnerDeletion, o2[i].BlockOwnerDeletion)
}

return equal
}

func boolPtrsEqual(b1, b2 *bool) bool {
if b1 == nil || b2 == nil {
return b1 == b2
}

return *b1 == *b2
}
2 changes: 2 additions & 0 deletions cns/restserver/restserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ func (service *HTTPRestService) Init(config *common.ServiceConfig) error {
listener.AddHandler(cns.NetworkContainersURLPath, service.getOrRefreshNetworkContainers)
listener.AddHandler(cns.GetHomeAz, service.getHomeAz)
listener.AddHandler(cns.EndpointPath, service.EndpointHandlerAPI)
listener.AddHandler(cns.IBDevicesPath, service.ibDevicesHandler)
// This API is only needed for Direct channel mode.
if config.ChannelMode == cns.Direct {
listener.AddHandler(cns.GetVMUniqueID, service.getVMUniqueID)
Expand Down Expand Up @@ -329,6 +330,7 @@ func (service *HTTPRestService) Init(config *common.ServiceConfig) error {
listener.AddHandler(cns.V2Prefix+cns.NmAgentSupportedApisPath, service.nmAgentSupportedApisHandler)
listener.AddHandler(cns.V2Prefix+cns.GetHomeAz, service.getHomeAz)
listener.AddHandler(cns.V2Prefix+cns.EndpointPath, service.EndpointHandlerAPI)
listener.AddHandler(cns.V2Prefix+cns.IBDevicesPath, service.ibDevicesHandler)
// This API is only needed for Direct channel mode.
if config.ChannelMode == cns.Direct {
listener.AddHandler(cns.V2Prefix+cns.GetVMUniqueID, service.getVMUniqueID)
Expand Down
Loading
Loading