pr suggestion updates

Author: MikeZappa87

pkg/adaptation/adaptation.go

@@ -240,21 +240,39 @@ func (r *Adaptation) UpdatePodSandbox(ctx context.Context, req *UpdatePodSandbox
 
 // PodSandboxStatus relays the corresponding CRI request to plugins.
 // If a plugin returns IP addresses, those will be returned to the caller.
-// The last plugin to return non-empty IPs wins.
+// An error is returned if multiple plugins attempt to set the same field.
 func (r *Adaptation) PodSandboxStatus(ctx context.Context, req *PodSandboxStatusRequest) (*PodSandboxStatusResponse, error) {
 	r.Lock()
 	defer r.Unlock()
 	defer r.removeClosedPlugins()
 
-	rsp := &PodSandboxStatusResponse{}
+	var (
+		rsp                = &PodSandboxStatusResponse{}
+		ipOwner            = ""
+		additionalIpsOwner = ""
+	)
+
 	for _, plugin := range r.plugins {
 		pluginRsp, err := plugin.podSandboxStatus(ctx, req)
 		if err != nil {
 			return nil, err
 		}
-		// Use the last plugin's non-empty IP response
-		if pluginRsp != nil && (pluginRsp.Ip != "" || len(pluginRsp.AdditionalIps) > 0) {
-			rsp = pluginRsp
+		if pluginRsp == nil || (pluginRsp.Ip == "" && len(pluginRsp.AdditionalIps) == 0) {
+			continue
+		}
+		if pluginRsp.Ip != "" {
+			if ipOwner != "" {
+				return nil, fmt.Errorf("plugins %q and %q both tried to set PodSandboxStatus IP address field", ipOwner, plugin.name())
+			}
+			rsp.Ip = pluginRsp.Ip
+			ipOwner = plugin.name()
+		}
+		if len(pluginRsp.AdditionalIps) > 0 {
+			if additionalIpsOwner != "" {
+				return nil, fmt.Errorf("plugins %q and %q both tried to set PodSandboxStatus additional IP addresses field", additionalIpsOwner, plugin.name())
+			}
+			rsp.AdditionalIps = pluginRsp.AdditionalIps
+			additionalIpsOwner = plugin.name()
 		}
 	}
 

pkg/stub/stub.go

@@ -96,9 +96,9 @@ type PostUpdatePodInterface interface {
 // PodSandboxStatusInterface handles PodSandboxStatus API requests.
 type PodSandboxStatusInterface interface {
 	// PodSandboxStatus relays a PodSandboxStatus request to the plugin.
-	// The plugin can return modified IP addresses for the pod sandbox.
-	// Returns: primary IP, additional IPs, error
-	PodSandboxStatus(context.Context, *api.PodSandbox) (string, []string, error)
+	// The plugin can return modified IP addresses for the pod sandbox
+	// in a PodSandboxStatusResponse.
+	PodSandboxStatus(context.Context, *api.PodSandbox) (*api.PodSandboxStatusResponse, error)
 }
 
 // CreateContainerInterface handles CreateContainer API requests.
@@ -324,7 +324,7 @@ type handlers struct {
 	StopPodSandbox              func(context.Context, *api.PodSandbox) error
 	RemovePodSandbox            func(context.Context, *api.PodSandbox) error
 	PostUpdatePodSandbox        func(context.Context, *api.PodSandbox) error
-	PodSandboxStatus            func(context.Context, *api.PodSandbox) (string, []string, error)
+	PodSandboxStatus            func(context.Context, *api.PodSandbox) (*api.PodSandboxStatusResponse, error)
 	CreateContainer             func(context.Context, *api.PodSandbox, *api.Container) (*api.ContainerAdjustment, []*api.ContainerUpdate, error)
 	StartContainer              func(context.Context, *api.PodSandbox, *api.Container) error
 	UpdateContainer             func(context.Context, *api.PodSandbox, *api.Container, *api.LinuxResources) ([]*api.ContainerUpdate, error)
@@ -797,11 +797,8 @@ func (stub *stub) PodSandboxStatus(ctx context.Context, req *api.PodSandboxStatu
 	if handler == nil {
 		return &api.PodSandboxStatusResponse{}, nil
 	}
-	ip, additionalIPs, err := handler(ctx, req.Pod)
-	return &api.PodSandboxStatusResponse{
-		Ip:            ip,
-		AdditionalIps: additionalIPs,
-	}, err
+
+	return handler(ctx, req.Pod)
 }
 
 // StateChange event handler.