Skip to content

Commit 242a045

Browse files
authored
fix(init): avoid duplicated okdev manifest paths (#69)
1 parent ad850e5 commit 242a045

2 files changed

Lines changed: 109 additions & 9 deletions

File tree

internal/cli/init.go

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -469,11 +469,7 @@ func scaffoldInitWorkload(configPath, templateRef string, vars *config.TemplateV
469469
if templatePath == "" || strings.TrimSpace(vars.ManifestPath) == "" {
470470
return nil, nil
471471
}
472-
target := vars.ManifestPath
473-
if !filepath.IsAbs(target) {
474-
target = filepath.Join(config.ManifestDir(configPath), target)
475-
}
476-
target = filepath.Clean(target)
472+
target := resolveInitScaffoldFilePath(configPath, vars.ManifestPath)
477473
if _, err := os.Stat(target); err == nil && !force {
478474
return nil, nil
479475
}
@@ -512,10 +508,7 @@ func scaffoldInitTemplateFiles(configPath, templateRef string, meta *config.Temp
512508
if target == "" {
513509
return nil, fmt.Errorf("template file path %q rendered empty", pathTemplate)
514510
}
515-
if !filepath.IsAbs(target) {
516-
target = filepath.Join(config.ManifestDir(configPath), target)
517-
}
518-
target = filepath.Clean(target)
511+
target = resolveInitScaffoldFilePath(configPath, target)
519512
if _, err := os.Stat(target); err == nil && !force {
520513
continue
521514
}
@@ -539,6 +532,22 @@ func scaffoldInitTemplateFiles(configPath, templateRef string, meta *config.Temp
539532
return wrote, nil
540533
}
541534

535+
func resolveInitScaffoldFilePath(configPath, path string) string {
536+
target := strings.TrimSpace(path)
537+
if filepath.IsAbs(target) {
538+
return filepath.Clean(target)
539+
}
540+
if isFolderConfigPath(configPath) && pathStartsWithDotOkdev(target) {
541+
return filepath.Clean(filepath.Join(config.RootDir(configPath), target))
542+
}
543+
return filepath.Clean(filepath.Join(config.ManifestDir(configPath), target))
544+
}
545+
546+
func pathStartsWithDotOkdev(path string) bool {
547+
cleaned := filepath.Clean(strings.TrimSpace(path))
548+
return cleaned == ".okdev" || strings.HasPrefix(cleaned, ".okdev"+string(filepath.Separator))
549+
}
550+
542551
func usesBuiltinBasicTemplate(ref string) bool {
543552
return strings.TrimSpace(ref) == "" || strings.TrimSpace(ref) == "basic"
544553
}

internal/cli/init_test.go

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,25 @@ func TestInitScaffoldsPyTorchJobManifest(t *testing.T) {
463463
}
464464
}
465465

466+
func TestInitScaffoldsPyTorchJobManifestWithFolderConfigAndDotOkdevPath(t *testing.T) {
467+
tmp := t.TempDir()
468+
opts := &Options{ConfigPath: filepath.Join(tmp, ".okdev", "okdev.yaml")}
469+
cmd := newInitCmd(opts)
470+
cmd.SetArgs([]string{"--yes", "--workload", "pytorchjob", "--manifest-path", ".okdev/pytorchjob.yaml"})
471+
cmd.SetIn(strings.NewReader(""))
472+
473+
if err := cmd.Execute(); err != nil {
474+
t.Fatalf("init execute: %v", err)
475+
}
476+
477+
if _, err := os.Stat(filepath.Join(tmp, ".okdev", "pytorchjob.yaml")); err != nil {
478+
t.Fatalf("expected manifest beside folder config, got err=%v", err)
479+
}
480+
if _, err := os.Stat(filepath.Join(tmp, ".okdev", ".okdev", "pytorchjob.yaml")); !os.IsNotExist(err) {
481+
t.Fatalf("expected duplicate .okdev manifest path to be absent, err=%v", err)
482+
}
483+
}
484+
466485
func TestInitManifestPreservedWithoutForce(t *testing.T) {
467486
tmp := t.TempDir()
468487
okdevDir := filepath.Join(tmp, ".okdev")
@@ -759,6 +778,78 @@ spec:
759778
}
760779
}
761780

781+
func TestInitCustomTemplateRendersDeclaredFileWithFolderConfigAndDotOkdevPath(t *testing.T) {
782+
tmp := t.TempDir()
783+
tmplDir := filepath.Join(tmp, ".okdev", "templates")
784+
if err := os.MkdirAll(filepath.Join(tmplDir, "manifests"), 0o755); err != nil {
785+
t.Fatal(err)
786+
}
787+
if err := os.WriteFile(filepath.Join(tmplDir, "pytorch.yaml.tmpl"), []byte(`---
788+
name: pytorch
789+
files:
790+
- path: "{{ .ManifestPath }}"
791+
template: manifests/pytorchjob.yaml.tmpl
792+
---
793+
apiVersion: okdev.io/v1alpha1
794+
kind: DevEnvironment
795+
metadata:
796+
name: {{ .Name }}
797+
spec:
798+
namespace: {{ .Namespace }}
799+
sync:
800+
paths:
801+
- "{{ .SyncLocal }}:{{ .SyncRemote }}"
802+
ssh:
803+
user: root
804+
sidecar:
805+
image: ghcr.io/acmore/okdev:edge
806+
workload:
807+
type: {{ .WorkloadType }}
808+
manifestPath: {{ .ManifestPath }}
809+
inject:
810+
- path: "spec.pytorchReplicaSpecs.Master.template"
811+
- path: "spec.pytorchReplicaSpecs.Worker.template"
812+
`), 0o644); err != nil {
813+
t.Fatal(err)
814+
}
815+
if err := os.WriteFile(filepath.Join(tmplDir, "manifests", "pytorchjob.yaml.tmpl"), []byte(`apiVersion: kubeflow.org/v1
816+
kind: PyTorchJob
817+
metadata:
818+
name: {{ .Name }}
819+
spec:
820+
pytorchReplicaSpecs:
821+
Master:
822+
template:
823+
spec:
824+
containers:
825+
- name: dev
826+
image: ubuntu:22.04
827+
Worker:
828+
template:
829+
spec:
830+
containers:
831+
- name: dev
832+
image: ubuntu:22.04
833+
`), 0o644); err != nil {
834+
t.Fatal(err)
835+
}
836+
837+
opts := &Options{ConfigPath: filepath.Join(tmp, ".okdev", "okdev.yaml")}
838+
cmd := newInitCmd(opts)
839+
cmd.SetArgs([]string{"--yes", "--template", "pytorch", "--workload", "pytorchjob", "--manifest-path", ".okdev/pytorchjob.yaml"})
840+
cmd.SetIn(strings.NewReader(""))
841+
842+
if err := cmd.Execute(); err != nil {
843+
t.Fatalf("init execute: %v", err)
844+
}
845+
if _, err := os.Stat(filepath.Join(tmp, ".okdev", "pytorchjob.yaml")); err != nil {
846+
t.Fatalf("expected companion manifest beside folder config, got err=%v", err)
847+
}
848+
if _, err := os.Stat(filepath.Join(tmp, ".okdev", ".okdev", "pytorchjob.yaml")); !os.IsNotExist(err) {
849+
t.Fatalf("expected duplicate .okdev companion path to be absent, err=%v", err)
850+
}
851+
}
852+
762853
func TestInitProjectTemplateShadowingBasicIsNotTreatedAsBuiltin(t *testing.T) {
763854
tmp := t.TempDir()
764855
tmplDir := filepath.Join(tmp, ".okdev", "templates")

0 commit comments

Comments
 (0)