Skip to content

Commit

Permalink
Fix nodetask.File dependency on owner
Browse files Browse the repository at this point in the history
  • Loading branch information
johngmyers committed May 24, 2020
1 parent e6d73b5 commit 9cd4516
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 17 deletions.
3 changes: 0 additions & 3 deletions upup/pkg/fi/nodeup/nodetasks/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ go_library(
"file.go",
"group.go",
"load_image.go",
"mount_disk.go",
"package.go",
"service.go",
"update_packages.go",
Expand All @@ -29,8 +28,6 @@ go_library(
"//util/pkg/hashing:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//vendor/k8s.io/klog:go_default_library",
"//vendor/k8s.io/utils/exec:go_default_library",
"//vendor/k8s.io/utils/mount:go_default_library",
],
)

Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/nodeup/nodetasks/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ var _ fi.HasDependencies = &File{}
func (e *File) GetDependencies(tasks map[string]fi.Task) []fi.Task {
var deps []fi.Task
if e.Owner != nil {
ownerTask := tasks["user/"+*e.Owner]
ownerTask := tasks["UserTask/"+*e.Owner]
if ownerTask == nil {
// The user might be a pre-existing user (e.g. admin)
klog.Warningf("Unable to find task %q", "user/"+*e.Owner)
Expand Down
61 changes: 48 additions & 13 deletions upup/pkg/fi/nodeup/nodetasks/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,39 @@ func TestFileDependencies(t *testing.T) {
childFileName := "/dependent"

grid := []struct {
name string
parent fi.Task
child fi.Task
}{
{
name: "user",
parent: &UserTask{
Name: "owner",
UID: 3,
Shell: "/bin/shell",
Home: "/home/owner",
},
child: &File{
Owner: fi.String("owner"),
Path: childFileName,
Contents: fi.NewStringResource("I depend on an owner"),
Type: FileType_File,
},
},
{
name: "parentDir",
parent: &File{
Path: parentFileName,
Type: FileType_Directory,
},
child: &File{
Path: parentFileName + "/" + childFileName,
Contents: fi.NewStringResource("I depend on my parent directory"),
Type: FileType_File,
},
},
{
name: "afterFiles",
parent: &File{
Path: parentFileName,
Contents: fi.NewStringResource("I am depended on by " + childFileName),
Expand All @@ -46,18 +75,24 @@ func TestFileDependencies(t *testing.T) {
}

for _, g := range grid {
allTasks := make(map[string]fi.Task)
allTasks["parent"] = g.parent
allTasks["child"] = g.child

deps := g.parent.(fi.HasDependencies).GetDependencies(allTasks)
if len(deps) != 0 {
t.Errorf("found unexpected dependencies for parent: %v %v", g.parent, deps)
}

childDeps := g.child.(fi.HasDependencies).GetDependencies(allTasks)
if len(childDeps) != 1 {
t.Errorf("found unexpected dependencies for child: %v %v", g.child, childDeps)
}
t.Run(g.name, func(t *testing.T) {
context := &fi.ModelBuilderContext{
Tasks: make(map[string]fi.Task),
}
context.AddTask(g.parent)
context.AddTask(g.child)

if _, ok := g.parent.(fi.HasDependencies); ok {
deps := g.parent.(fi.HasDependencies).GetDependencies(context.Tasks)
if len(deps) != 0 {
t.Errorf("found unexpected dependencies for parent: %v %v", g.parent, deps)
}
}

childDeps := g.child.(fi.HasDependencies).GetDependencies(context.Tasks)
if len(childDeps) != 1 {
t.Errorf("found unexpected dependencies for child: %v %v", g.child, childDeps)
}
})
}
}

0 comments on commit 9cd4516

Please sign in to comment.