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

chore: watch flag skips files specified by dockerignore #5565

Merged
merged 22 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ require (
github.com/imdario/mergo v0.3.16
github.com/lnquy/cron v1.1.1
github.com/moby/buildkit v0.11.6
github.com/moby/patternmatcher v0.6.0
github.com/onsi/ginkgo/v2 v2.13.2
github.com/onsi/gomega v1.30.0
github.com/robfig/cron/v3 v3.0.1
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b h1:j7+1HpAFS1zy5+Q4qx1f
github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE=
github.com/moby/buildkit v0.11.6 h1:VYNdoKk5TVxN7k4RvZgdeM4GOyRvIi4Z8MXOY7xvyUs=
github.com/moby/buildkit v0.11.6/go.mod h1:GCqKfHhz+pddzfgaR7WmHVEE3nKKZMMDPpK8mh3ZLv4=
github.com/moby/patternmatcher v0.6.0 h1:GmP9lR19aU5GqSSFko+5pRqHi+Ohk1O69aFiKkVGiPk=
github.com/moby/patternmatcher v0.6.0/go.mod h1:hDPoyOpDY7OrrMDLaYoY3hf52gNCR/YOUYxkhApJIxc=
github.com/onsi/ginkgo/v2 v2.13.2 h1:Bi2gGVkfn6gQcjNjZJVO8Gf0FHzMPf2phUei9tejVMs=
github.com/onsi/ginkgo/v2 v2.13.2/go.mod h1:XStQ8QcGwLyF4HdfcZB8SFOS/MWCgDuXMSBe6zrvLgM=
github.com/onsi/gomega v1.30.0 h1:hvMK7xYz4D3HapigLTeGdId/NcfQx1VHMJc60ew99+8=
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/cli/flag.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ Format: [container]:KEY=VALUE. Omit container name to apply to all containers.`
Example: --port-override 5000:80 binds localhost:5000 to the service's port 80.`
proxyFlagDescription = `Optional. Proxy outbound requests to your environment's VPC.`
proxyNetworkFlagDescription = `Optional. Set the IP Network used by --proxy.`
watchFlagDescription = `Optional. Watch changes to local files and restart containers when updated.`
watchFlagDescription = `Optional. Watch changes to local files and restart containers when updated. Directories and files in the main .dockerignore file are ignored.`
useTaskRoleFlagDescription = "Optional. Run containers with TaskRole credentials instead of session credentials."

svcManifestFlagDescription = `Optional. Name of the environment in which the service was deployed;
Expand Down
4 changes: 4 additions & 0 deletions internal/pkg/cli/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -754,3 +754,7 @@ type stackConfiguration interface {
type secretGetter interface {
GetSecretValue(context.Context, string) (string, error)
}

type dockerWorkload interface {
Dockerfile() string
}
37 changes: 37 additions & 0 deletions internal/pkg/cli/mocks/mock_interfaces.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

96 changes: 70 additions & 26 deletions internal/pkg/cli/run_local.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import (
"github.com/aws/copilot-cli/internal/pkg/deploy/cloudformation"
"github.com/aws/copilot-cli/internal/pkg/describe"
"github.com/aws/copilot-cli/internal/pkg/docker/dockerengine"
"github.com/aws/copilot-cli/internal/pkg/docker/dockerfile"
"github.com/aws/copilot-cli/internal/pkg/docker/orchestrator"
"github.com/aws/copilot-cli/internal/pkg/ecs"
"github.com/aws/copilot-cli/internal/pkg/exec"
Expand Down Expand Up @@ -116,28 +117,29 @@ type runLocalVars struct {
type runLocalOpts struct {
runLocalVars

sel deploySelector
ecsClient ecsClient
ecsExecutor ecsCommandExecutor
ssm secretGetter
secretsManager secretGetter
sessProvider sessionProvider
sess *session.Session
envManagerSess *session.Session
targetEnv *config.Environment
targetApp *config.Application
store store
ws wsWlDirReader
cmd execRunner
dockerEngine dockerEngineRunner
repository repositoryService
prog progress
orchestrator containerOrchestrator
hostFinder hostFinder
envChecker versionCompatibilityChecker
debounceTime time.Duration
newRecursiveWatcher func() (recursiveWatcher, error)

sel deploySelector
ecsClient ecsClient
ecsExecutor ecsCommandExecutor
ssm secretGetter
secretsManager secretGetter
sessProvider sessionProvider
sess *session.Session
envManagerSess *session.Session
targetEnv *config.Environment
targetApp *config.Application
store store
ws wsWlDirReader
cmd execRunner
dockerEngine dockerEngineRunner
repository repositoryService
prog progress
orchestrator containerOrchestrator
hostFinder hostFinder
envChecker versionCompatibilityChecker
debounceTime time.Duration
dockerExcludes []string

newRecursiveWatcher func() (recursiveWatcher, error)
buildContainerImages func(mft manifest.DynamicWorkload) (map[string]string, error)
configureClients func() error
labeledTermPrinter func(fw syncbuffer.FileWriter, bufs []*syncbuffer.LabeledSyncBuffer, opts ...syncbuffer.LabeledTermPrinterOption) clideploy.LabeledTermPrinter
Expand Down Expand Up @@ -237,6 +239,16 @@ func newRunLocalOpts(vars runLocalVars) (*runLocalOpts, error) {
return nil
}
o.buildContainerImages = func(mft manifest.DynamicWorkload) (map[string]string, error) {
var dfDir string
if dockerWkld, ok := mft.Manifest().(dockerWorkload); ok {
CaptainCarpensir marked this conversation as resolved.
Show resolved Hide resolved
dfDir = filepath.Dir(dockerWkld.Dockerfile())
}
o.dockerExcludes, err = dockerfile.ReadDockerignore(afero.NewOsFs(), filepath.Join(ws.Path(), dfDir))
if err != nil {
return nil, err
}
o.filterDockerExcludes()

gitShortCommit := imageTagFromGit(o.cmd)
image := clideploy.ContainerImageIdentifier{
GitShortCommitTag: gitShortCommit,
Expand Down Expand Up @@ -593,6 +605,20 @@ func (o *runLocalOpts) prepareTask(ctx context.Context) (orchestrator.Task, erro
return task, nil
}

func (o *runLocalOpts) filterDockerExcludes() {
wsPath := o.ws.Path()
result := []string{}

// filter out excludes to the copilot directory, we always want to watch these files
for _, exclude := range o.dockerExcludes {
if !strings.HasPrefix(filepath.ToSlash(exclude), filepath.ToSlash(filepath.Join(wsPath, workspace.CopilotDirName))) {
CaptainCarpensir marked this conversation as resolved.
Show resolved Hide resolved
result = append(result, exclude)
}
}

o.dockerExcludes = result
}

func (o *runLocalOpts) watchLocalFiles(stopCh <-chan struct{}) (<-chan interface{}, <-chan error, error) {
workspacePath := o.ws.Path()

Expand All @@ -612,6 +638,7 @@ func (o *runLocalOpts) watchLocalFiles(stopCh <-chan struct{}) (<-chan interface
watcherErrors := watcher.Errors()

debounceTimer := time.NewTimer(o.debounceTime)
debounceTimerRunning := false
if !debounceTimer.Stop() {
// flush the timer in case stop is called after the timer finishes
<-debounceTimer.C
Expand Down Expand Up @@ -640,11 +667,12 @@ func (o *runLocalOpts) watchLocalFiles(stopCh <-chan struct{}) (<-chan interface
break
}

// check if any subdirectories within copilot directory are hidden
isHidden := false
parent := workspacePath
suffix, _ := strings.CutPrefix(event.Name, parent+"/")

// check if any subdirectories within copilot directory are hidden
// fsnotify events are always of form /a/b/c, don't use filepath.Split as that's OS dependent
isHidden := false
for _, child := range strings.Split(suffix, "/") {
parent = filepath.Join(parent, child)
subdirHidden, err := file.IsHiddenFile(child)
Expand All @@ -656,11 +684,27 @@ func (o *runLocalOpts) watchLocalFiles(stopCh <-chan struct{}) (<-chan interface
}
}

// TODO(Aiden): implement dockerignore blacklist for update
if !isHidden {
// skip updates from files matching .dockerignore patterns
isExcluded := false
for _, pattern := range o.dockerExcludes {
CaptainCarpensir marked this conversation as resolved.
Show resolved Hide resolved
matches, err := filepath.Match(pattern, suffix)
if err != nil {
break
}
if matches {
isExcluded = true
}
}

if !isHidden && !isExcluded {
if !debounceTimerRunning {
fmt.Println("Restarting task...")
debounceTimerRunning = true
}
debounceTimer.Reset(o.debounceTime)
}
case <-debounceTimer.C:
debounceTimerRunning = false
watchCh <- nil
}
}
Expand Down
105 changes: 77 additions & 28 deletions internal/pkg/cli/run_local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,16 +483,17 @@ func TestRunLocalOpts_Execute(t *testing.T) {
}

testCases := map[string]struct {
inputAppName string
inputEnvName string
inputWkldName string
inputEnvOverrides map[string]string
inputPortOverrides []string
inputWatch bool
inputTaskRole bool
inputProxy bool
inputReader io.Reader
buildImagesError error
inputAppName string
inputEnvName string
inputWkldName string
inputEnvOverrides map[string]string
inputPortOverrides []string
inputDockerExcludes []string
inputWatch bool
inputTaskRole bool
inputProxy bool
inputReader io.Reader
buildImagesError error

setupMocks func(t *testing.T, m *runLocalExecuteMocks)
wantedWkldName string
Expand Down Expand Up @@ -948,33 +949,38 @@ ecs exec: all containers failed to retrieve credentials`),
}
},
},
"watch flag receives hidden file update, doesn't restart": {
inputAppName: testAppName,
inputWkldName: testWkldName,
inputEnvName: testEnvName,
inputWatch: true,
"watch flag receives hidden file and ignored file update, doesn't restart": {
inputAppName: testAppName,
inputWkldName: testWkldName,
inputEnvName: testEnvName,
inputWatch: true,
inputDockerExcludes: []string{"ignoredDir/*"},
setupMocks: func(t *testing.T, m *runLocalExecuteMocks) {
m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil)
m.ssm.EXPECT().GetSecretValue(gomock.Any(), "mysecret").Return("secretvalue", nil)
m.ws.EXPECT().ReadWorkloadManifest(testWkldName).Return([]byte(""), nil)
m.interpolator.EXPECT().Interpolate("").Return("", nil)
m.ws.EXPECT().Path().Return("")

eventCh := make(chan fsnotify.Event, 1)
eventCh := make(chan fsnotify.Event, 2)
m.watcher.EventsFn = func() <-chan fsnotify.Event {
eventCh <- fsnotify.Event{
Name: ".hiddensubdir/mockFilename",
Op: fsnotify.Write,
}
eventCh <- fsnotify.Event{
Name: "ignoredDir/mockFilename",
Op: fsnotify.Write,
}
return eventCh
}

watcherErrCh := make(chan error, 1)
watcherErrCh := make(chan error, 2)
m.watcher.ErrorsFn = func() <-chan error {
return watcherErrCh
}

errCh := make(chan error, 1)
errCh := make(chan error, 2)
m.orchestrator.StartFn = func() <-chan error {
return errCh
}
Expand Down Expand Up @@ -1192,16 +1198,17 @@ ecs exec: all containers failed to retrieve credentials`),
Credentials: credentials.NewStaticCredentials("myEnvID", "myEnvSecret", "myEnvToken"),
},
},
cmd: m.mockRunner,
dockerEngine: m.dockerEngine,
repository: m.repository,
targetEnv: &mockEnv,
targetApp: &mockApp,
prog: m.prog,
orchestrator: m.orchestrator,
hostFinder: m.hostFinder,
envChecker: m.envChecker,
debounceTime: 0, // disable debounce during testing
cmd: m.mockRunner,
dockerEngine: m.dockerEngine,
repository: m.repository,
targetEnv: &mockEnv,
targetApp: &mockApp,
prog: m.prog,
orchestrator: m.orchestrator,
hostFinder: m.hostFinder,
envChecker: m.envChecker,
debounceTime: 0, // disable debounce during testing
dockerExcludes: tc.inputDockerExcludes,
newRecursiveWatcher: func() (recursiveWatcher, error) {
return m.watcher, nil
},
Expand Down Expand Up @@ -1971,3 +1978,45 @@ func TestRunLocal_HostDiscovery(t *testing.T) {
})
}
}

type runLocalFilterDockerExcludesMocks struct {
ws *mocks.MockwsWlDirReader
}

func TestRunLocal_FilterDockerExcludes(t *testing.T) {
tests := map[string]struct {
setupMocks func(t *testing.T, m *runLocalFilterDockerExcludesMocks)

inputDockerExcludes []string
wantedDockerExcludes []string
}{
"filter out all copilot directories": {
setupMocks: func(t *testing.T, m *runLocalFilterDockerExcludesMocks) {
m.ws.EXPECT().Path().Return("/ws")
},
inputDockerExcludes: []string{"/ws/copilot/*", "/ws/ignoredfile.go", "/ws/copilot/environments/*"},
wantedDockerExcludes: []string{"/ws/ignoredfile.go"},
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
// GIVEN
ctrl := gomock.NewController(t)
defer ctrl.Finish()
m := &runLocalFilterDockerExcludesMocks{
ws: mocks.NewMockwsWlDirReader(ctrl),
}
tc.setupMocks(t, m)
opts := runLocalOpts{
dockerExcludes: tc.inputDockerExcludes,
ws: m.ws,
}

// WHEN
opts.filterDockerExcludes()

// THEN
require.Equal(t, opts.dockerExcludes, tc.wantedDockerExcludes)
})
}
}