sandbox: merge address and protocol to one url

abel-von · abel-von · commit de38490ed6b8 · 2024-04-30T15:28:00.000+08:00
Signed-off-by: Abel Feng &lt;fshb1988@gmail.com&gt;
diff --git a/api/next.pb.txt b/api/next.pb.txt
@@ -5520,13 +5520,6 @@ file {
       type: TYPE_UINT32
       json_name: "version"
     }
-    field {
-      name: "protocol"
-      number: 7
-      label: LABEL_OPTIONAL
-      type: TYPE_STRING
-      json_name: "protocol"
-    }
     nested_type {
       name: "LabelsEntry"
       field {
@@ -5731,13 +5724,6 @@ file {
       type: TYPE_UINT32
       json_name: "version"
     }
-    field {
-      name: "protocol"
-      number: 10
-      label: LABEL_OPTIONAL
-      type: TYPE_STRING
-      json_name: "protocol"
-    }
     nested_type {
       name: "InfoEntry"
       field {
diff --git a/api/services/sandbox/v1/sandbox.pb.go b/api/services/sandbox/v1/sandbox.pb.go
diff --git a/api/services/sandbox/v1/sandbox.proto b/api/services/sandbox/v1/sandbox.proto
@@ -120,9 +120,11 @@ message ControllerStartResponse {
 	uint32 pid = 2;
 	google.protobuf.Timestamp created_at = 3;
 	map<string, string> labels = 4;
+	// Address of the sandbox for containerd to connect,
+	// for calling Task or other APIs serving in the sandbox.
+	// it is in the form of ttrpc+unix://path/to/uds or grpc+vsock://<vsock cid>:<port>.
 	string address = 5;
 	uint32 version = 6;
-	string protocol = 7;
 }
 
 message ControllerPlatformRequest {
@@ -166,9 +168,11 @@ message ControllerStatusResponse {
 	google.protobuf.Timestamp created_at = 5;
 	google.protobuf.Timestamp exited_at = 6;
 	google.protobuf.Any extra = 7;
+	// Address of the sandbox for containerd to connect,
+	// for calling Task or other APIs serving in the sandbox.
+	// it is in the form of ttrpc+unix://path/to/uds or grpc+vsock://<vsock cid>:<port>.
 	string address = 8;
 	uint32 version = 9;
-	string protocol = 10;
 }
 
 message ControllerShutdownRequest {
diff --git a/client/task_opts.go b/client/task_opts.go
@@ -52,7 +52,7 @@ func WithRuntimePath(absRuntimePath string) NewTaskOpts {
 
 // WithTaskAPIEndpoint allow task service to manage a task through a given endpoint,
 // usually it is served inside a sandbox, and we can get it from sandbox status.
-func WithTaskAPIEndpoint(address, protocol string, version uint32) NewTaskOpts {
+func WithTaskAPIEndpoint(address string, version uint32) NewTaskOpts {
 	return func(ctx context.Context, client *Client, info *TaskInfo) error {
 		if info.Options == nil {
 			info.Options = &options.Options{}
@@ -62,7 +62,6 @@ func WithTaskAPIEndpoint(address, protocol string, version uint32) NewTaskOpts {
 			return errors.New("invalid runtime v2 options format")
 		}
 		opts.TaskApiAddress = address
-		opts.TaskApiProtocol = protocol
 		opts.TaskApiVersion = version
 		return nil
 	}
diff --git a/core/runtime/runtime.go b/core/runtime/runtime.go
@@ -53,8 +53,6 @@ type CreateOpts struct {
 	SandboxID string
 	// Address is an optional Address for Task API server
 	Address string
-	// Protocol is an optional Protocol for Task API connection
-	Protocol string
 	// Version is an optional Version of the Task API
 	Version uint32
 }
diff --git a/core/runtime/v2/binary.go b/core/runtime/v2/binary.go
@@ -143,10 +143,12 @@ func (b *binary) Start(ctx context.Context, opts *types.Any, onClose func()) (_
 	if err := writeBootstrapParams(filepath.Join(b.bundle.Path, "bootstrap.json"), params); err != nil {
 		return nil, fmt.Errorf("failed to write bootstrap.json: %w", err)
 	}
-
+	// The address is in the form like ttrpc+unix://<uds-path> or grpc+vsock://<cid>:<port>
+	address := fmt.Sprintf("%s+%s", params.Protocol, params.Address)
 	return &shim{
 		bundle:  b.bundle,
 		client:  conn,
+		address: address,
 		version: params.Version,
 	}, nil
 }
diff --git a/core/runtime/v2/runc/options/next.pb.txt b/core/runtime/v2/runc/options/next.pb.txt
@@ -80,16 +80,9 @@ file {
       type: TYPE_STRING
       json_name: "taskApiAddress"
     }
-    field {
-      name: "task_api_protocol"
-      number: 13
-      label: LABEL_OPTIONAL
-      type: TYPE_STRING
-      json_name: "taskApiProtocol"
-    }
     field {
       name: "task_api_version"
-      number: 14
+      number: 13
       label: LABEL_OPTIONAL
       type: TYPE_UINT32
       json_name: "taskApiVersion"
diff --git a/core/runtime/v2/runc/options/oci.pb.go b/core/runtime/v2/runc/options/oci.pb.go
diff --git a/core/runtime/v2/runc/options/oci.proto b/core/runtime/v2/runc/options/oci.proto
@@ -30,11 +30,10 @@ message Options {
 	// criu work path
 	string criu_work_path = 11;
 	// task api address, can be a unix domain socket, or vsock address.
+	// it is in the form of ttrpc+unix://path/to/uds or grpc+vsock://<vsock cid>:<port>.
 	string task_api_address = 12;
-	// protocol for task api connection, currently ttrpc and grpc supported
-	string task_api_protocol = 13;
 	// task api version, currently supported value is 2 and 3.
-	uint32 task_api_version = 14;
+	uint32 task_api_version = 13;
 }
 
 message CheckpointOptions {
diff --git a/core/runtime/v2/shim.go b/core/runtime/v2/shim.go
@@ -113,9 +113,13 @@ func loadShim(ctx context.Context, bundle *Bundle, onClose func()) (_ ShimInstan
 		}
 	}()
 
+	// The address is in the form like ttrpc+unix://<uds-path> or grpc+vsock://<cid>:<port>
+	address := fmt.Sprintf("%s+%s", params.Protocol, params.Address)
+
 	shim := &shim{
 		bundle:  bundle,
 		client:  conn,
+		address: address,
 		version: params.Version,
 	}
 
@@ -186,8 +190,8 @@ type ShimInstance interface {
 	// Delete will close the client and remove bundle from disk.
 	Delete(ctx context.Context) error
 	// Endpoint returns shim's endpoint information,
-	// including address, protocol and version.
-	Endpoint() (string, string, int)
+	// including address and version.
+	Endpoint() (string, int)
 }
 
 func parseStartResponse(response []byte) (client.BootstrapParams, error) {
@@ -360,11 +364,10 @@ func (gc *grpcConn) UserOnCloseWait(ctx context.Context) error {
 }
 
 type shim struct {
-	bundle   *Bundle
-	client   any
-	address  string
-	protocol string
-	version  int
+	bundle  *Bundle
+	client  any
+	address string
+	version int
 }
 
 var _ ShimInstance = (*shim)(nil)
@@ -374,8 +377,8 @@ func (s *shim) ID() string {
 	return s.bundle.ID
 }
 
-func (s *shim) Endpoint() (string, string, int) {
-	return s.address, s.protocol, s.version
+func (s *shim) Endpoint() (string, int) {
+	return s.address, s.version
 }
 
 func (s *shim) Namespace() string {
@@ -443,7 +446,7 @@ type shimTask struct {
 }
 
 func newShimTask(shim ShimInstance) (*shimTask, error) {
-	_, _, version := shim.Endpoint()
+	_, version := shim.Endpoint()
 	taskClient, err := NewTaskClient(shim.Client(), version)
 	if err != nil {
 		return nil, err
diff --git a/core/runtime/v2/shim_manager.go b/core/runtime/v2/shim_manager.go
@@ -164,16 +164,27 @@ func (m *ShimManager) Start(ctx context.Context, id string, bundle *Bundle, opts
 	// This container belongs to sandbox which supposed to be already started via sandbox API.
 	if opts.SandboxID != "" {
 		var params shimbinary.BootstrapParams
-		if opts.Address != "" && opts.Protocol != "" {
+		if opts.Address != "" {
+			// The address returned from sandbox controller should be in the form like ttrpc+unix://<uds-path>
+			// or grpc+vsock://<cid>:<port>, we should get the protocol from the url first.
+			protocol, address, ok := strings.Cut(opts.Address, "+")
+			if !ok {
+				return nil, fmt.Errorf("the scheme of sandbox address should be in " +
+					" the form of <protocol>+<unix|vsock|tcp>, i.e. ttrpc+unix or grpc+vsock")
+			}
 			params = shimbinary.BootstrapParams{
 				Version:  int(opts.Version),
-				Address:  opts.Address,
-				Protocol: opts.Protocol,
+				Protocol: protocol,
+				Address:  address,
 			}
 		} else {
 			// For those sandbox we can not get endpoint,
 			// fallback to legacy implementation
-			p, restoreErr := m.restoreBootstrapParams(ctx, opts.SandboxID)
+			process, err := m.Get(ctx, opts.SandboxID)
+			if err != nil {
+				return nil, fmt.Errorf("can't find sandbox %s", opts.SandboxID)
+			}
+			p, restoreErr := restoreBootstrapParams(process.Bundle())
 			if restoreErr != nil {
 				return nil, fmt.Errorf("failed to get bootstrap "+
 					"params of sandbox %s, %v, legacy restore error %v", opts.SandboxID, err, restoreErr)
@@ -259,18 +270,6 @@ func (m *ShimManager) startShim(ctx context.Context, bundle *Bundle, id string,
 	return shim, nil
 }
 
-func (m *ShimManager) restoreBootstrapParams(ctx context.Context, sandboxID string) (shimbinary.BootstrapParams, error) {
-	process, err := m.Get(ctx, sandboxID)
-	if err != nil {
-		return shimbinary.BootstrapParams{}, fmt.Errorf("can't find sandbox %s", sandboxID)
-	}
-	params, err := restoreBootstrapParams(filepath.Join(m.state, process.Namespace(), sandboxID))
-	if err != nil {
-		return shimbinary.BootstrapParams{}, err
-	}
-	return params, nil
-}
-
 // restoreBootstrapParams reads bootstrap.json to restore shim configuration.
 // If its an old shim, this will perform migration - read address file and write default bootstrap
 // configuration (version = 2, protocol = ttrpc, and address).
diff --git a/core/sandbox/controller.go b/core/sandbox/controller.go
@@ -118,7 +118,6 @@ type ControllerInstance struct {
 	Pid       uint32
 	CreatedAt time.Time
 	Address   string
-	Protocol  string
 	Version   uint32
 	Labels    map[string]string
 }
@@ -137,6 +136,5 @@ type ControllerStatus struct {
 	ExitedAt  time.Time
 	Extra     typeurl.Any
 	Address   string
-	Protocol  string
 	Version   uint32
 }
diff --git a/core/sandbox/proxy/controller.go b/core/sandbox/proxy/controller.go
@@ -74,7 +74,6 @@ func (s *remoteSandboxController) Start(ctx context.Context, sandboxID string) (
 		CreatedAt: resp.GetCreatedAt().AsTime(),
 		Labels:    resp.GetLabels(),
 		Address:   resp.GetAddress(),
-		Protocol:  resp.GetProtocol(),
 		Version:   resp.GetVersion(),
 	}, nil
 }
@@ -145,7 +144,6 @@ func (s *remoteSandboxController) Status(ctx context.Context, sandboxID string,
 		ExitedAt:  resp.GetExitedAt().AsTime(),
 		Extra:     resp.GetExtra(),
 		Address:   resp.GetAddress(),
-		Protocol:  resp.GetProtocol(),
 		Version:   resp.GetVersion(),
 	}, nil
 }
diff --git a/internal/cri/server/container_start.go b/internal/cri/server/container_start.go
@@ -123,7 +123,7 @@ func (c *criService) StartContainer(ctx context.Context, r *runtime.StartContain
 	endpoint := sandbox.Endpoint
 	if endpoint.IsValid() {
 		taskOpts = append(taskOpts,
-			containerd.WithTaskAPIEndpoint(endpoint.Address, endpoint.Protocol, endpoint.Version))
+			containerd.WithTaskAPIEndpoint(endpoint.Address, endpoint.Version))
 	}
 
 	task, err := container.NewTask(ctx, ioCreation, taskOpts...)
diff --git a/internal/cri/server/restart.go b/internal/cri/server/restart.go
@@ -127,7 +127,6 @@ func (c *criService) recover(ctx context.Context) error {
 				state = sandboxstore.StateNotReady
 			}
 		} else {
-			endpoint.Protocol = status.Protocol
 			endpoint.Version = status.Version
 			endpoint.Address = status.Address
 			if code, ok := runtime.PodSandboxState_value[status.State]; ok {
diff --git a/internal/cri/server/sandbox_run.go b/internal/cri/server/sandbox_run.go
diff --git a/internal/cri/store/sandbox/sandbox.go b/internal/cri/store/sandbox/sandbox.go
diff --git a/plugins/sandbox/controller.go b/plugins/sandbox/controller.go
diff --git a/plugins/services/tasks/local.go b/plugins/services/tasks/local.go

Original file line number	Diff line number	Diff line change
`@@ -53,8 +53,6 @@ type CreateOpts struct {`
`53`	`53`	`SandboxID string`
`54`	`54`	`// Address is an optional Address for Task API server`
`55`	`55`	`Address string`
`56`		`- // Protocol is an optional Protocol for Task API connection`
`57`		`- Protocol string`
`58`	`56`	`// Version is an optional Version of the Task API`
`59`	`57`	`Version uint32`
`60`	`58`	`}`
Original file line number	Diff line number	Diff line change
`@@ -143,10 +143,12 @@ func (b binary) Start(ctx context.Context, opts types.Any, onClose func()) (_`
`143`	`143`	`if err := writeBootstrapParams(filepath.Join(b.bundle.Path, "bootstrap.json"), params); err != nil {`
`144`	`144`	`return nil, fmt.Errorf("failed to write bootstrap.json: %w", err)`
`145`	`145`	`}`
`146`		`-`
	`146`	`+ // The address is in the form like ttrpc+unix://<uds-path> or grpc+vsock://<cid>:<port>`
	`147`	`+ address := fmt.Sprintf("%s+%s", params.Protocol, params.Address)`
`147`	`148`	`return &shim{`
`148`	`149`	`bundle: b.bundle,`
`149`	`150`	`client: conn,`
	`151`	`+ address: address,`
`150`	`152`	`version: params.Version,`
`151`	`153`	`}, nil`
`152`	`154`	`}`