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

Make the file sync destination more consistent #3662

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
6 changes: 3 additions & 3 deletions docs/public/debugging-using-devfile.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,19 @@ commands:
- type: exec
component: runtime
command: "npm install"
workdir: ${CHE_PROJECTS_ROOT}/nodejs-starter/app
workdir: ${PROJECTS_ROOT}/nodejs-starter/app
- name: devrun
actions:
- type: exec
component: runtime
command: "nodemon app.js"
workdir: ${CHE_PROJECTS_ROOT}/nodejs-starter/app
workdir: ${PROJECTS_ROOT}/nodejs-starter/app
- name: debugrun
actions:
- type: exec
component: runtime
command: "nodemon --inspect=${DEBUG_PORT}"
workdir: ${CHE_PROJECTS_ROOT}/nodejs-starter/
workdir: ${PROJECTS_ROOT}/nodejs-starter/
```

- Now we need to create the component using `odo create nodejs`
Expand Down
15 changes: 13 additions & 2 deletions pkg/devfile/adapters/common/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ const (
// Default volume size for volumes defined in a devfile
volumeSize = "5Gi"

// EnvCheProjectsRoot is the env defined for /projects where component mountSources=true
EnvCheProjectsRoot = "CHE_PROJECTS_ROOT"
// EnvProjectsRoot is the env defined for /projects where component mountSources=true
EnvProjectsRoot = "PROJECTS_ROOT"

// EnvOdoCommandRunWorkingDir is the env defined in the runtime component container which holds the work dir for the run command
EnvOdoCommandRunWorkingDir = "ODO_COMMAND_RUN_WORKING_DIR"
Expand Down Expand Up @@ -235,3 +235,14 @@ func GetCommandsMap(commands []common.DevfileCommand) map[string]common.DevfileC
}
return commandsMap
}

// GetComponentEnvVar returns true if a list of env vars contains the specified env var
// If the env exists, it returns the value of it
func GetComponentEnvVar(env string, envs []common.Env) string {
for _, envVar := range envs {
if envVar.Name == env {
return envVar.Value
}
}
return ""
}
65 changes: 65 additions & 0 deletions pkg/devfile/adapters/common/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -587,3 +587,68 @@ func TestGetCommandsMap(t *testing.T) {
}

}

func TestGetComponentEnvVar(t *testing.T) {

tests := []struct {
name string
env string
envs []common.Env
want string
}{
{
name: "Case 1: No env vars",
env: "test",
envs: nil,
want: "",
},
{
name: "Case 2: Has env",
env: "PROJECTS_ROOT",
envs: []common.Env{
{
Name: "SOME_ENV",
Value: "test",
},
{
Name: "TESTER",
Value: "tester",
},
{
Name: "PROJECTS_ROOT",
Value: "/test",
},
},
want: "/test",
},
{
name: "Case 3: No env, multiple values",
env: "PROJECTS_ROOT",
envs: []common.Env{
{
Name: "TESTER",
Value: "fake",
},
{
Name: "FAKE",
Value: "fake",
},
{
Name: "ENV",
Value: "fake",
},
},
want: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
value := GetComponentEnvVar(tt.env, tt.envs)
if value != tt.want {
t.Errorf("TestGetComponentEnvVar error: env value mismatch, expected: %v got: %v", tt.want, value)
}

})
}

}
17 changes: 10 additions & 7 deletions pkg/devfile/adapters/docker/component/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,17 +198,20 @@ func (a Adapter) startComponent(mounts []mount.Mount, comp versionsCommon.Devfil
}
updateComponentWithSupervisord(&comp, runCommand, a.supervisordVolumeName, &hostConfig)

// If the component set `mountSources` to true, add the source volume and env CHE_PROJECTS_ROOT to it
// If the component set `mountSources` to true, add the source volume and env PROJECTS_ROOT to it
if comp.Container.MountSources {
var syncFolder, projectsRoot string
if comp.Container.SourceMapping != "" {
utils.AddVolumeToContainer(a.projectVolumeName, comp.Container.SourceMapping, &hostConfig)
syncFolder = comp.Container.SourceMapping
} else if projectsRoot = common.GetComponentEnvVar(common.EnvProjectsRoot, comp.Container.Env); projectsRoot != "" {
syncFolder = projectsRoot
} else {
utils.AddVolumeToContainer(a.projectVolumeName, lclient.OdoSourceVolumeMount, &hostConfig)
syncFolder = lclient.OdoSourceVolumeMount
}

if !common.IsEnvPresent(comp.Container.Env, common.EnvCheProjectsRoot) {
envName := common.EnvCheProjectsRoot
envValue := lclient.OdoSourceVolumeMount
utils.AddVolumeToContainer(a.projectVolumeName, syncFolder, &hostConfig)
if projectsRoot != "" {
envName := common.EnvProjectsRoot
envValue := syncFolder
comp.Container.Env = append(comp.Container.Env, versionsCommon.Env{
Name: envName,
Value: envValue,
Expand Down
8 changes: 5 additions & 3 deletions pkg/devfile/adapters/kubernetes/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,11 @@ func GetContainers(devfileObj devfileParser.DevfileObj) ([]corev1.Container, err
// If `mountSources: true` was set, add an empty dir volume to the container to sync the source to
// Sync to `Container.SourceMapping` if set
if comp.Container.MountSources {
var syncFolder string
var syncFolder, projectsRoot string
if comp.Container.SourceMapping != "" {
syncFolder = comp.Container.SourceMapping
} else if projectsRoot = adaptersCommon.GetComponentEnvVar(adaptersCommon.EnvProjectsRoot, comp.Container.Env); projectsRoot != "" {
syncFolder = projectsRoot
} else {
syncFolder = kclient.OdoSourceVolumeMount
}
Expand All @@ -96,10 +98,10 @@ func GetContainers(devfileObj devfileParser.DevfileObj) ([]corev1.Container, err
})

// only add the env if it is not set by the devfile
if !isEnvPresent(container.Env, adaptersCommon.EnvCheProjectsRoot) {
if projectsRoot == "" {
container.Env = append(container.Env,
corev1.EnvVar{
Name: adaptersCommon.EnvCheProjectsRoot,
Name: adaptersCommon.EnvProjectsRoot,
Value: syncFolder,
})
}
Expand Down
38 changes: 1 addition & 37 deletions pkg/sync/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"strings"

"github.com/openshift/odo/pkg/devfile/adapters/common"
versionsCommon "github.com/openshift/odo/pkg/devfile/parser/data/common"
"github.com/openshift/odo/pkg/exec"
"github.com/openshift/odo/pkg/kclient"
"github.com/openshift/odo/pkg/log"
Expand Down Expand Up @@ -157,17 +156,7 @@ func (a Adapter) pushLocal(path string, files []string, delFiles []string, isFor
s := log.Spinner("Syncing files to the component")
defer s.End(false)

// Determine which folder we need to sync to
var syncFolder string
if compInfo.SourceMount != kclient.OdoSourceVolumeMount {
syncFolder = compInfo.SourceMount
} else {
// If there's only one project defined in the devfile, sync to `/projects/project-name`, otherwise sync to /projects
syncFolder, err = getSyncFolder(a.Devfile.Data.GetProjects())
if err != nil {
return errors.Wrapf(err, "unable to determine sync folder")
}
}
syncFolder := compInfo.SourceMount

if syncFolder != kclient.OdoSourceVolumeMount {
// Need to make sure the folder already exists on the component or else sync will fail
Expand Down Expand Up @@ -210,31 +199,6 @@ func (a Adapter) pushLocal(path string, files []string, delFiles []string, isFor
return nil
}

// getSyncFolder returns the folder that we need to sync the source files to
// If there's exactly one project defined in the devfile, and clonePath isn't set return `/projects/<projectName>`
// If there's exactly one project, and clonePath is set, return `/projects/<clonePath>`
// If the clonePath is an absolute path or contains '..', return an error
// Otherwise (zero projects or many), return `/projects`
func getSyncFolder(projects []versionsCommon.DevfileProject) (string, error) {
if len(projects) == 1 {
project := projects[0]
// If the clonepath is set to a value, set it to be the sync folder
// As some devfiles rely on the code being synced to the folder in the clonepath
if project.ClonePath != "" {
if strings.HasPrefix(project.ClonePath, "/") {
return "", fmt.Errorf("the clonePath in the devfile must be a relative path")
}
if strings.Contains(project.ClonePath, "..") {
return "", fmt.Errorf("the clonePath in the devfile cannot escape the projects root. Don't use .. to try and do that")
}
return filepath.ToSlash(filepath.Join(kclient.OdoSourceVolumeMount, project.ClonePath)), nil
}
return filepath.ToSlash(filepath.Join(kclient.OdoSourceVolumeMount, projects[0].Name)), nil
}
return kclient.OdoSourceVolumeMount, nil

}

// getCmdToCreateSyncFolder returns the command used to create the remote sync folder on the running container
func getCmdToCreateSyncFolder(syncFolder string) []string {
return []string{"mkdir", "-p", syncFolder}
Expand Down
126 changes: 0 additions & 126 deletions pkg/sync/adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,132 +17,6 @@ import (
"github.com/openshift/odo/tests/helper"
)

func TestGetSyncFolder(t *testing.T) {
projectNames := []string{"some-name", "another-name"}
projectRepos := []string{"https://github.com/some/repo.git", "https://github.com/another/repo.git"}
projectClonePath := "src/github.com/golang/example/"
invalidClonePaths := []string{"/var", "../var", "pkg/../../var"}

tests := []struct {
name string
projects []versionsCommon.DevfileProject
want string
wantErr bool
}{
{
name: "Case 1: No projects",
projects: []versionsCommon.DevfileProject{},
want: kclient.OdoSourceVolumeMount,
wantErr: false,
},
{
name: "Case 2: One project",
projects: []versionsCommon.DevfileProject{
{
Name: projectNames[0],
Git: &versionsCommon.Git{
Location: projectRepos[0],
},
},
},
want: filepath.ToSlash(filepath.Join(kclient.OdoSourceVolumeMount, projectNames[0])),
wantErr: false,
},
{
name: "Case 3: Multiple projects",
projects: []versionsCommon.DevfileProject{
{
Name: projectNames[0],
Git: &versionsCommon.Git{
Location: projectRepos[0],
},
},
{
Name: projectNames[1],
Github: &versionsCommon.Github{
Location: projectRepos[1],
},
},
{
Name: projectNames[1],
Zip: &versionsCommon.Zip{
Location: projectRepos[1],
},
},
},
want: kclient.OdoSourceVolumeMount,
wantErr: false,
},
{
name: "Case 4: Clone path set",
projects: []versionsCommon.DevfileProject{
{
ClonePath: projectClonePath,
Name: projectNames[0],
Zip: &versionsCommon.Zip{
Location: projectRepos[0],
},
},
},
want: filepath.ToSlash(filepath.Join(kclient.OdoSourceVolumeMount, projectClonePath)),
wantErr: false,
},
{
name: "Case 5: Invalid clone path, set with absolute path",
projects: []versionsCommon.DevfileProject{
{
ClonePath: invalidClonePaths[0],
Name: projectNames[0],
Github: &versionsCommon.Github{
Location: projectRepos[0],
},
},
},
want: "",
wantErr: true,
},
{
name: "Case 6: Invalid clone path, starts with ..",
projects: []versionsCommon.DevfileProject{
{
ClonePath: invalidClonePaths[1],
Name: projectNames[0],
Git: &versionsCommon.Git{
Location: projectRepos[0],
},
},
},
want: "",
wantErr: true,
},
{
name: "Case 7: Invalid clone path, contains ..",
projects: []versionsCommon.DevfileProject{
{
ClonePath: invalidClonePaths[2],
Name: projectNames[0],
Zip: &versionsCommon.Zip{
Location: projectRepos[0],
},
},
},
want: "",
wantErr: true,
},
}
for _, tt := range tests {
syncFolder, err := getSyncFolder(tt.projects)

if !tt.wantErr == (err != nil) {
t.Errorf("expected %v, actual %v", tt.wantErr, err)
}

if syncFolder != tt.want {
t.Errorf("expected %s, actual %s", tt.want, syncFolder)
}
}
}

func TestGetCmdToCreateSyncFolder(t *testing.T) {
tests := []struct {
name string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ commands:
id: devbuild
component: runtime
commandLine: npm install
workingDir: ${CHE_PROJECTS_ROOT}/nodejs-starter
workingDir: ${PROJECTS_ROOT}
group:
kind: build
isDefault: true
- exec:
id: devrun
component: runtime
commandLine: npm start
workingDir: ${CHE_PROJECTS_ROOT}/nodejs-starter
workingDir: ${PROJECTS_ROOT}
group:
kind: run
isDefault: true
Loading