Skip to content
Merged
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
40 changes: 28 additions & 12 deletions pkg/oss/nodeserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,18 @@ func validateNodePublishVolumeRequest(req *csi.NodePublishVolumeRequest) error {
return nil
}

// Runtime types when using csi-agent: rund & MicroVM
// Runtime types when using proxy mounter: runc & rund
// Runtime types when using cmd mounter: MicroVM
//
// opts.DirectAssigned is configured via PV attributes to declare whether skipAttach is needed. true: COCO or rund
// Explanation: opts.DirectAssigned was originally used to declare COCO, and later extended to distinguish
// runc&rund mixed deployment scenarios, where true means rund, false means runc
// Note: opts.DirectAssigned defaults to false, and only has meaning when true. When false, it may represent
// various runtime types other than COCO depending on different runtime environments
// ns.skipAttach: nodeserver configuration exclusive to csi-agent binary. true: rund or MicroVM
// socketPath: socket path used to communicate with proxy mounter. non-empty: runc or rund

func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) {
klog.Infof("NodePublishVolume:: Starting Mount volume: %s", req.VolumeId)
if !ns.locks.TryAcquire(req.VolumeId) {
Expand Down Expand Up @@ -128,15 +140,24 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
return nil, status.Error(codes.InvalidArgument, err.Error())
}

socketPath := req.PublishContext[mountProxySocket]

// Determine runtime type based on directAssigned, socketPath, and skipAttach
// See DetermineRuntimeType for the support matrix.
runtimeType, err := DetermineRuntimeType(opts.DirectAssigned, socketPath, ns.skipAttach)
if err != nil {
return nil, status.Errorf(codes.InvalidArgument, "failed to determine runtime type: %v", err)
}
klog.V(4).InfoS("Determined runtime type", "runtimeType", runtimeType, "directAssigned", opts.DirectAssigned, "hasSocketPath", socketPath != "", "skipAttach", ns.skipAttach)

// check and make auth config
authCfg, err := makeAuthConfig(opts, ns.fusePodManagers[opts.FuseType], ns.metadata, true)
if err != nil {
return nil, status.Error(codes.InvalidArgument, err.Error())
}

// Note: In non-csi-agent environment (!ns.skipAttach),
// if DirectAssigned is True, it's a confidential container scenario (coco)
if opts.DirectAssigned && !ns.skipAttach {
// Handle COCO scenario: directAssigned=true, skipAttach=false
if runtimeType == RuntimeTypeCOCO {
return ns.publishDirectVolume(ctx, req, opts)
}

Expand All @@ -149,26 +170,21 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
}
mountOptions = ns.fusePodManagers[opts.FuseType].AddDefaultMountOptions(mountOptions)

// get mount proxy socket path
socketPath := req.PublishContext[mountProxySocket]
if socketPath == "" && !ns.skipAttach {
return nil, status.Errorf(codes.InvalidArgument, "%s not found in publishContext", mountProxySocket)
}

// Note: In ACK and ACS GPU scenarios, the socket path is provided by publishContext.
var ossfsMounter mounter.Mounter
if socketPath == "" {
if runtimeType == RuntimeTypeMicroVM {
mountOptions, err = ossfpm.AppendRRSAAuthOptions(ns.metadata, mountOptions, req.VolumeId, targetPath, authCfg)
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
ossfsMounter = mounter.NewOssCmdMounter(ns.ossfsPaths[opts.FuseType], req.VolumeId, ns.rawMounter)
} else {
// runtimeType == RuntimeTypeRunD || runtimeType == RuntimeTypeRunC
ossfsMounter = mounter.NewProxyMounter(socketPath, ns.rawMounter)
}

// When work as csi-agent, directly mount on the target path.
if ns.skipAttach {
if runtimeType == RuntimeTypeRunD || runtimeType == RuntimeTypeMicroVM {
metricsPath := utils.WriteMetricsInfo(metricsPathPrefix, req, opts.MetricsTop, opts.FuseType, "oss", opts.Bucket)
err := ossfsMounter.ExtendedMount(
mountSource, targetPath, opts.FuseType,
Expand All @@ -181,7 +197,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
}
klog.Infof("NodePublishVolume(csi-agent): successfully mounted %s on %s", mountSource, targetPath)
return &csi.NodePublishVolumeResponse{}, nil
}
} // else: runtimeType == RuntimeTypeRunC

// When work as csi nodeserver, mount on the attach path under /run/fuse.ossfs and then perform the bind mount.
// check whether the attach path is mounted
Expand Down
91 changes: 91 additions & 0 deletions pkg/oss/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,97 @@ func parseDirectAssigned(runtimeClassValue, directAssignedValue string) bool {
return result
}

// RuntimeType represents the container runtime type
type RuntimeType string

const (
RuntimeTypeCOCO RuntimeType = "coco"
RuntimeTypeRunD RuntimeType = "rund"
RuntimeTypeRunC RuntimeType = "runc"
RuntimeTypeMicroVM RuntimeType = "eci"
RuntimeTypeUnknown RuntimeType = "unknown"
)

// runtimeTypeResult represents the result of runtime type determination
type runtimeTypeResult struct {
runtimeType RuntimeType
err error
}

// runtimeTypeLookupTable is a lookup table for all 8 possible combinations of
// (directAssigned, hasSocketPath, skipAttach).
// Index calculation: index = (directAssigned ? 4 : 0) + (hasSocketPath ? 2 : 0) + (skipAttach ? 1 : 0)å
var runtimeTypeLookupTable = [8]runtimeTypeResult{
// Index 0: directAssigned=false, hasSocketPath=false, skipAttach=false -> error (should not occur)
{
runtimeType: RuntimeTypeUnknown,
err: fmt.Errorf("socket path is empty in non csi-agent mode (should not occur)"),
},
// Index 1: directAssigned=false, hasSocketPath=false, skipAttach=true -> MicroVM
{
runtimeType: RuntimeTypeMicroVM,
err: nil,
},
// Index 2: directAssigned=false, hasSocketPath=true, skipAttach=false -> RunC
{
runtimeType: RuntimeTypeRunC,
err: nil,
},
// Index 3: directAssigned=false, hasSocketPath=true, skipAttach=true -> RunD (pure rund cluster, no need to specify directAssigned)
{
runtimeType: RuntimeTypeRunD,
err: nil,
},
// Index 4: directAssigned=true, hasSocketPath=false, skipAttach=false -> COCO
{
runtimeType: RuntimeTypeCOCO,
err: nil,
},
// Index 5: directAssigned=true, hasSocketPath=false, skipAttach=true -> MicroVM (directAssigned not effective)
{
runtimeType: RuntimeTypeMicroVM,
err: nil,
},
// Index 6: directAssigned=true, hasSocketPath=true, skipAttach=false -> error (should not occur, controller ensures empty socketPath for COCO)
{
runtimeType: RuntimeTypeUnknown,
err: fmt.Errorf("rund cannot be used in non csi-agent mode (should not occur)"),
},
// Index 7: directAssigned=true, hasSocketPath=true, skipAttach=true -> RunD
{
runtimeType: RuntimeTypeRunD,
err: nil,
},
}

// DetermineRuntimeType determines the container runtime type based on directAssigned, socketPath, and skipAttach.
//
// This function uses a table-driven approach with a 3-bit index calculated from the three boolean parameters.
// The lookup table contains all 8 possible combinations, providing O(1) lookup performance.
//
// Returns:
// - RuntimeType: the determined runtime type
// - error: if the combination is invalid (should not occur scenarios)
func DetermineRuntimeType(directAssigned bool, socketPath string, skipAttach bool) (RuntimeType, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't like this change. We don't actually care about the name of each runtime. We should have an abstract level to extract what is really different between these runtimes, and write code based on those differences. We can add comments about typical runtime, but don't depends on the runtime name in code.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The actual difference is 'skipAttach, directAssigned amd socketPath'. I write down the meaning of them but It's still difficult to manage the code if we don't name the runtime type clearly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think skipAttach should be renamed to something like agentMode. Most branches should already be self-explanatory.

The only difficult case is

	// Note: In non-csi-agent environment (!ns.skipAttach),
	//   if DirectAssigned is True, it's a confidential container scenario (coco)
	if opts.DirectAssigned && !ns.skipAttach {
		return ns.publishDirectVolume(ctx, req, opts)
	}

and we already have comments about it.

Particularly, socketPath == "" is a lot better than runtimeType == RuntimeTypeMicroVM, because the former clearly tells us we don't have mount proxy for this case.

Similarly, ns.agentMode (renamed from skipAttach) should be better than runtimeType == RuntimeTypeRunD || runtimeType == RuntimeTypeMicroVM. When run as agent, we don't need to care about globalmount.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If the logic is more difficult? It's hard to manage the code. I dont want to spend minute to consider every if-else.

hasSocketPath := socketPath != ""

// Calculate index: (directAssigned ? 4 : 0) + (hasSocketPath ? 2 : 0) + (skipAttach ? 1 : 0)
index := 0
if directAssigned {
index += 4
}
if hasSocketPath {
index += 2
}
if skipAttach {
index += 1
}

// Lookup result from table
result := runtimeTypeLookupTable[index]
return result.runtimeType, result.err
}

// getDirectAssignedValue returns the default value for DirectAssigned option based on the runtime class.
// The function reads DEFAULT_RUNTIME_CLASS environment variable and determines the appropriate default value:
// - For "rund" runtime, returns true (direct assignment enabled by default)
Expand Down
93 changes: 93 additions & 0 deletions pkg/oss/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1119,3 +1119,96 @@ func TestParseDirectAssigned(t *testing.T) {
})
}
}

func TestDetermineRuntimeType(t *testing.T) {
tests := []struct {
name string
directAssigned bool
socketPath string
skipAttach bool
wantRuntimeType RuntimeType
wantError bool
errorContains string
}{
{
name: "COCO: directAssigned=true, socketPath=empty, skipAttach=false",
directAssigned: true,
socketPath: "",
skipAttach: false,
wantRuntimeType: RuntimeTypeCOCO,
wantError: false,
},
{
name: "RunD: directAssigned=true, socketPath=non-empty, skipAttach=true",
directAssigned: true,
socketPath: "/path/to/socket",
skipAttach: true,
wantRuntimeType: RuntimeTypeRunD,
wantError: false,
},
{
name: "RunD: directAssigned=false, socketPath=non-empty, skipAttach=true",
directAssigned: false,
socketPath: "/path/to/socket",
skipAttach: true,
wantRuntimeType: RuntimeTypeRunD,
wantError: false,
},
{
name: "RunC: directAssigned=false, socketPath=non-empty, skipAttach=false",
directAssigned: false,
socketPath: "/path/to/socket",
skipAttach: false,
wantRuntimeType: RuntimeTypeRunC,
wantError: false,
},
{
name: "MicroVM: directAssigned=true, socketPath=empty, skipAttach=true",
directAssigned: true,
socketPath: "",
skipAttach: true,
wantRuntimeType: RuntimeTypeMicroVM,
wantError: false,
},
{
name: "MicroVM: directAssigned=false, socketPath=empty, skipAttach=true",
directAssigned: false,
socketPath: "",
skipAttach: true,
wantRuntimeType: RuntimeTypeMicroVM,
wantError: false,
},
{
name: "Invalid: directAssigned=true, socketPath=non-empty, skipAttach=false",
directAssigned: true,
socketPath: "/path/to/socket",
skipAttach: false,
wantError: true,
errorContains: "should not occur",
},
{
name: "Invalid: directAssigned=false, socketPath=empty, skipAttach=false",
directAssigned: false,
socketPath: "",
skipAttach: false,
wantError: true,
errorContains: "should not occur",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotRuntimeType, err := DetermineRuntimeType(tt.directAssigned, tt.socketPath, tt.skipAttach)
if tt.wantError {
assert.Error(t, err)
if tt.errorContains != "" {
assert.Contains(t, err.Error(), tt.errorContains)
}
assert.Equal(t, RuntimeTypeUnknown, gotRuntimeType)
} else {
assert.NoError(t, err)
assert.Equal(t, tt.wantRuntimeType, gotRuntimeType)
}
})
}
}