From aed53999e33b537de5bf89f5015ff7f5c716c7ab Mon Sep 17 00:00:00 2001 From: Md Raiyan Date: Mon, 25 May 2026 05:12:01 +0000 Subject: [PATCH] refactor(shim): unify config.json access via PatchConfig Signed-off-by: Md Raiyan --- internal/constants/constants.go | 2 + pkg/containerd-shim/containerd/annotations.go | 97 +++---------------- pkg/containerd-shim/containerd/config.go | 89 +++++++++++++++++ pkg/containerd-shim/guest_rootfs.go | 90 +++++++---------- pkg/unikontainers/rootfs.go | 4 - pkg/unikontainers/unikontainers.go | 3 +- 6 files changed, 141 insertions(+), 144 deletions(-) create mode 100644 pkg/containerd-shim/containerd/config.go diff --git a/internal/constants/constants.go b/internal/constants/constants.go index b15132ca9..526105347 100644 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -15,3 +15,5 @@ package constants const TimestampTargetFile = "/tmp/urunc.zlog" + +const AnnotRootfsParams = "com.urunc.internal.rootfs.params" diff --git a/pkg/containerd-shim/containerd/annotations.go b/pkg/containerd-shim/containerd/annotations.go index 5d980c961..023bcaf02 100644 --- a/pkg/containerd-shim/containerd/annotations.go +++ b/pkg/containerd-shim/containerd/annotations.go @@ -19,8 +19,6 @@ import ( "encoding/json" "fmt" "io" - "os" - "path/filepath" "strings" contentapi "github.com/containerd/containerd/api/services/content/v1" @@ -86,7 +84,18 @@ func InjectUruncAnnotations(ctx context.Context, session *Session, bundlePath st return nil } - return patchConfigJSON(bundlePath, annotations) + return PatchConfig(bundlePath, func(spec *runtimespec.Spec) error { + if spec.Annotations == nil { + spec.Annotations = make(map[string]string) + } + for k, v := range annotations { + if _, exists := spec.Annotations[k]; exists { + continue + } + spec.Annotations[k] = v + } + return nil + }) } func (f *annotationFetcher) fetchUruncAnnotations(ctx context.Context) (map[string]string, error) { @@ -151,85 +160,3 @@ func readBlob(ctx context.Context, namespace string, contentClient contentapi.Co return raw, nil } - -// patchConfigJSON injects missing annotations into the OCI runtime spec -// stored in the bundle's config.json. -// -// Existing annotations in config.json are preserved. Only annotation keys that -// are not already present in the runtime spec are added. -func patchConfigJSON(bundlePath string, annotations map[string]string) error { - configPath := filepath.Join(bundlePath, "config.json") - - fi, err := os.Stat(configPath) - if err != nil { - return fmt.Errorf("stat config.json: %w", err) - } - - data, err := os.ReadFile(configPath) - if err != nil { - return fmt.Errorf("read config.json: %w", err) - } - - var spec runtimespec.Spec - if err := json.Unmarshal(data, &spec); err != nil { - return fmt.Errorf("unmarshal spec: %w", err) - } - - if spec.Annotations == nil { - spec.Annotations = make(map[string]string) - } - - for k, v := range annotations { - if _, exists := spec.Annotations[k]; exists { - continue - } - spec.Annotations[k] = v - } - - patched, err := json.MarshalIndent(spec, "", " ") - if err != nil { - return fmt.Errorf("marshal spec: %w", err) - } - - if err := atomicWriteFile(configPath, patched, fi.Mode()); err != nil { - return fmt.Errorf("write config.json atomically: %w", err) - } - return nil -} - -func atomicWriteFile(path string, data []byte, mode os.FileMode) error { - tmpDir := filepath.Dir(path) - - f, err := os.CreateTemp(tmpDir, "."+filepath.Base(path)+".tmp-*") - if err != nil { - return err - } - - tmpName := f.Name() - defer os.Remove(tmpName) - - if err := f.Chmod(mode); err != nil { - _ = f.Close() - return err - } - - if _, err := f.Write(data); err != nil { - _ = f.Close() - return err - } - - if err := f.Sync(); err != nil { - _ = f.Close() - return err - } - - if err := f.Close(); err != nil { - return err - } - - if err := os.Rename(tmpName, path); err != nil { - return err - } - - return nil -} diff --git a/pkg/containerd-shim/containerd/config.go b/pkg/containerd-shim/containerd/config.go new file mode 100644 index 000000000..4e3622da5 --- /dev/null +++ b/pkg/containerd-shim/containerd/config.go @@ -0,0 +1,89 @@ +// Copyright (c) 2023-2026, Nubificus LTD +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package containerd + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + + runtimespec "github.com/opencontainers/runtime-spec/specs-go" +) + +// PatchConfig reads config.json from bundlePath, applies fn to the spec, +// and writes it back atomically. +func PatchConfig(bundlePath string, fn func(*runtimespec.Spec) error) error { + configPath := filepath.Join(bundlePath, "config.json") + + fi, err := os.Stat(configPath) + if err != nil { + return fmt.Errorf("stat config.json: %w", err) + } + + data, err := os.ReadFile(configPath) + if err != nil { + return fmt.Errorf("read config.json: %w", err) + } + + var spec runtimespec.Spec + if err := json.Unmarshal(data, &spec); err != nil { + return fmt.Errorf("unmarshal config.json: %w", err) + } + + if err := fn(&spec); err != nil { + return err + } + + patched, err := json.MarshalIndent(spec, "", " ") + if err != nil { + return fmt.Errorf("marshal config.json: %w", err) + } + + return atomicWriteFile(configPath, patched, fi.Mode()) +} + +func atomicWriteFile(path string, data []byte, mode os.FileMode) error { + tmpDir := filepath.Dir(path) + + f, err := os.CreateTemp(tmpDir, "."+filepath.Base(path)+".tmp-*") + if err != nil { + return err + } + + tmpName := f.Name() + defer os.Remove(tmpName) + + if err := f.Chmod(mode); err != nil { + _ = f.Close() + return err + } + + if _, err := f.Write(data); err != nil { + _ = f.Close() + return err + } + + if err := f.Sync(); err != nil { + _ = f.Close() + return err + } + + if err := f.Close(); err != nil { + return err + } + + return os.Rename(tmpName, path) +} diff --git a/pkg/containerd-shim/guest_rootfs.go b/pkg/containerd-shim/guest_rootfs.go index f8982ecf1..c81012f38 100644 --- a/pkg/containerd-shim/guest_rootfs.go +++ b/pkg/containerd-shim/guest_rootfs.go @@ -18,75 +18,57 @@ import ( "encoding/json" "errors" "fmt" - "os" "path/filepath" taskAPI "github.com/containerd/containerd/api/runtime/task/v2" specs "github.com/opencontainers/runtime-spec/specs-go" + + "github.com/urunc-dev/urunc/internal/constants" + containerdShim "github.com/urunc-dev/urunc/pkg/containerd-shim/containerd" "github.com/urunc-dev/urunc/pkg/unikontainers" ) -const annotRootfsParams = "com.urunc.internal.rootfs.params" - var errGuestRootfsChoiceSkipped = errors.New("guest rootfs choice skipped") // chooseGuestRootfs runs the same ChooseRootfs logic as runtime Exec after inner -// task Create (#684) and records the result in annotRootfsParams so Exec knows +// task Create and records the result in constants.AnnotRootfsParams so Exec knows // selection already happened. func chooseGuestRootfs(r *taskAPI.CreateTaskRequest) error { - configPath := filepath.Join(r.Bundle, "config.json") - info, err := os.Stat(configPath) - if err != nil { - return fmt.Errorf("stat config.json: %w", err) - } - - data, err := os.ReadFile(configPath) - if err != nil { - return fmt.Errorf("read config.json: %w", err) - } - - var spec specs.Spec - if err := json.Unmarshal(data, &spec); err != nil { - return fmt.Errorf("unmarshal config.json: %w", err) - } - if spec.Root == nil { - return fmt.Errorf("invalid OCI spec: root section is required") - } - - config, err := unikontainers.GetUnikernelConfig(filepath.Clean(r.Bundle), &spec) - if err != nil { - return fmt.Errorf("%w: %w", errGuestRootfsChoiceSkipped, err) - } + return containerdShim.PatchConfig(r.Bundle, func(spec *specs.Spec) error { + if spec.Root == nil { + return fmt.Errorf("invalid OCI spec: root section is required") + } - annotations := config.Map() - uruncCfg, err := unikontainers.LoadUruncConfig(unikontainers.UruncConfigPath) - if err != nil && uruncCfg == nil { - return err - } + config, err := unikontainers.GetUnikernelConfig(filepath.Clean(r.Bundle), spec) + if err != nil { + return fmt.Errorf("%w: %w", errGuestRootfsChoiceSkipped, err) + } - rootfsParams, err := unikontainers.ChooseRootfs( - filepath.Clean(r.Bundle), - spec.Root.Path, - annotations, - uruncCfg, - ) - if err != nil { - return err - } + annotations := config.Map() + uruncCfg, err := unikontainers.LoadUruncConfig(unikontainers.UruncConfigPath) + if err != nil && uruncCfg == nil { + return err + } - encoded, err := json.Marshal(rootfsParams) - if err != nil { - return err - } - if spec.Annotations == nil { - spec.Annotations = make(map[string]string) - } - spec.Annotations[annotRootfsParams] = string(encoded) + rootfsParams, err := unikontainers.ChooseRootfs( + filepath.Clean(r.Bundle), + spec.Root.Path, + annotations, + uruncCfg, + ) + if err != nil { + return err + } - patched, err := json.MarshalIndent(spec, "", " ") - if err != nil { - return fmt.Errorf("marshal config.json: %w", err) - } + encoded, err := json.Marshal(rootfsParams) + if err != nil { + return err + } - return os.WriteFile(configPath, patched, info.Mode()) + if spec.Annotations == nil { + spec.Annotations = make(map[string]string) + } + spec.Annotations[constants.AnnotRootfsParams] = string(encoded) + return nil + }) } diff --git a/pkg/unikontainers/rootfs.go b/pkg/unikontainers/rootfs.go index 0d7c80c0f..0fd7e38d9 100644 --- a/pkg/unikontainers/rootfs.go +++ b/pkg/unikontainers/rootfs.go @@ -28,10 +28,6 @@ import ( // TODO: Find and set the correct size for the tmpfs in the host const tmpfsSizeForNoRootfs = "65536k" -// annotRootfsParams holds JSON RootfsParams after shim chooseGuestRootfs. -// When present in bundle config.json, Exec reuses it; otherwise Exec runs ChooseRootfs. -const annotRootfsParams = "com.urunc.internal.rootfs.params" - type rootfsBuilder interface { preSetup() error postSetup() error diff --git a/pkg/unikontainers/unikontainers.go b/pkg/unikontainers/unikontainers.go index a3a00bb5f..62159cb1d 100644 --- a/pkg/unikontainers/unikontainers.go +++ b/pkg/unikontainers/unikontainers.go @@ -29,6 +29,7 @@ import ( "sync" "syscall" + "github.com/urunc-dev/urunc/internal/constants" "github.com/urunc-dev/urunc/pkg/network" "github.com/urunc-dev/urunc/pkg/unikontainers/hypervisors" "github.com/urunc-dev/urunc/pkg/unikontainers/types" @@ -433,7 +434,7 @@ func (u *Unikontainer) Exec(metrics m.Writer) error { var rootfsParams types.RootfsParams // Read the rootfs choice written by the shim. - if rootfsParamsJSON := u.Spec.Annotations[annotRootfsParams]; rootfsParamsJSON != "" { + if rootfsParamsJSON := u.Spec.Annotations[constants.AnnotRootfsParams]; rootfsParamsJSON != "" { if err := json.Unmarshal([]byte(rootfsParamsJSON), &rootfsParams); err != nil { return fmt.Errorf("could not decode guest rootfs params: %w", err) }