Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use unique key for jib caches #3483

Merged
merged 1 commit into from
Jan 9, 2020
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
2 changes: 1 addition & 1 deletion pkg/skaffold/build/jib/gradle.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (b *Builder) runGradleCommand(ctx context.Context, out io.Writer, workspace
// All paths are absolute.
func getDependenciesGradle(ctx context.Context, workspace string, a *latest.JibArtifact) ([]string, error) {
cmd := getCommandGradle(ctx, workspace, a)
deps, err := getDependencies(workspace, cmd, a.Project)
deps, err := getDependencies(workspace, cmd, a)
if err != nil {
return nil, errors.Wrapf(err, "getting jib-gradle dependencies")
}
Expand Down
18 changes: 12 additions & 6 deletions pkg/skaffold/build/jib/jib.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,16 @@ type filesLists struct {
}

// watchedFiles maps from project name to watched files
var watchedFiles = map[string]filesLists{}
var watchedFiles = map[projectKey]filesLists{}

func GetBuildDefinitions(a *latest.JibArtifact) []string {
return watchedFiles[a.Project].BuildDefinitions
type projectKey string

func getProjectKey(workspace string, a *latest.JibArtifact) projectKey {
return projectKey(workspace + "+" + a.Project)
Copy link
Member

Choose a reason for hiding this comment

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

As discussed, the fix in this PR make sense. I'll just let other Go folks to approve this particular implementation.

}

func GetBuildDefinitions(workspace string, a *latest.JibArtifact) []string {
return watchedFiles[getProjectKey(workspace, a)].BuildDefinitions
}

// GetDependencies returns a list of files to watch for changes to rebuild
Expand Down Expand Up @@ -132,9 +138,9 @@ func DeterminePluginType(workspace string, artifact *latest.JibArtifact) (Plugin
}

// getDependencies returns a list of files to watch for changes to rebuild
func getDependencies(workspace string, cmd exec.Cmd, projectName string) ([]string, error) {
func getDependencies(workspace string, cmd exec.Cmd, a *latest.JibArtifact) ([]string, error) {
var dependencyList []string
files, ok := watchedFiles[projectName]
files, ok := watchedFiles[getProjectKey(workspace, a)]
if !ok {
files = filesLists{}
}
Expand Down Expand Up @@ -175,7 +181,7 @@ func getDependencies(workspace string, cmd exec.Cmd, projectName string) ([]stri
}

// Store updated files list information
watchedFiles[projectName] = files
watchedFiles[getProjectKey(workspace, a)] = files

sort.Strings(dependencyList)
return dependencyList, nil
Expand Down
34 changes: 30 additions & 4 deletions pkg/skaffold/build/jib/jib_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func TestGetDependencies(t *testing.T) {
test.stdout,
))

results, err := getDependencies(tmpDir.Root(), exec.Cmd{Args: []string{"ignored"}, Dir: tmpDir.Root()}, util.RandomID())
results, err := getDependencies(tmpDir.Root(), exec.Cmd{Args: []string{"ignored"}, Dir: tmpDir.Root()}, &latest.JibArtifact{Project: util.RandomID()})

t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expectedDeps, results)
})
Expand All @@ -123,10 +123,10 @@ func TestGetUpdatedDependencies(t *testing.T) {
)

listCmd := exec.Cmd{Args: []string{"ignored"}, Dir: tmpDir.Root()}
projectID := util.RandomID()
artifact := &latest.JibArtifact{Project: util.RandomID()}

// List dependencies
_, err := getDependencies(tmpDir.Root(), listCmd, projectID)
_, err := getDependencies(tmpDir.Root(), listCmd, artifact)
t.CheckNoError(err)

// Create new build definition files
Expand All @@ -135,7 +135,7 @@ func TestGetUpdatedDependencies(t *testing.T) {
Write("settings.gradle", "")

// Update dependencies
_, err = getDependencies(tmpDir.Root(), listCmd, projectID)
_, err = getDependencies(tmpDir.Root(), listCmd, artifact)
t.CheckNoError(err)
})
}
Expand Down Expand Up @@ -193,3 +193,29 @@ func TestDeterminePluginType(t *testing.T) {
})
}
}

func TestGetProjectKey(t *testing.T) {
tests := []struct {
description string
artifact *latest.JibArtifact
workspace string
expected projectKey
}{
{
"empty project",
&latest.JibArtifact{},
"dir",
projectKey("dir+"),
},
{
"non-empty project",
&latest.JibArtifact{Project: "project"},
"dir",
projectKey("dir+project"),
},
}
for _, test := range tests {
projectKey := getProjectKey(test.workspace, test.artifact)
testutil.CheckDeepEqual(t, test.expected, projectKey)
}
}
2 changes: 1 addition & 1 deletion pkg/skaffold/build/jib/maven.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func (b *Builder) runMavenCommand(ctx context.Context, out io.Writer, workspace
// getDependenciesMaven finds the source dependencies for the given jib-maven artifact.
// All paths are absolute.
func getDependenciesMaven(ctx context.Context, workspace string, a *latest.JibArtifact) ([]string, error) {
deps, err := getDependencies(workspace, getCommandMaven(ctx, workspace, a), a.Project)
deps, err := getDependencies(workspace, getCommandMaven(ctx, workspace, a), a)
if err != nil {
return nil, errors.Wrapf(err, "getting jib-maven dependencies")
}
Expand Down