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

Don't unmute logs if an error happened #928

Merged
merged 2 commits into from
Sep 7, 2018
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
14 changes: 12 additions & 2 deletions pkg/skaffold/kubernetes/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type LogAggregator struct {

muted int32
startTime time.Time
cancel context.CancelFunc
trackedContainers trackedContainers
}

Expand All @@ -61,7 +62,11 @@ func NewLogAggregator(out io.Writer, podSelector PodSelector, colorPicker ColorP
}
}

// Start starts a logger that listens to pods and tail their logs
// if they are matched by the `podSelector`.
func (a *LogAggregator) Start(ctx context.Context) error {
cancelCtx, cancel := context.WithCancel(ctx)
a.cancel = cancel
a.startTime = time.Now()

kubeclient, err := Client()
Expand All @@ -85,7 +90,7 @@ func (a *LogAggregator) Start(ctx context.Context) error {

for {
select {
case <-ctx.Done():
case <-cancelCtx.Done():
return
case evt, ok := <-watcher.ResultChan():
if !ok {
Expand All @@ -102,7 +107,7 @@ func (a *LogAggregator) Start(ctx context.Context) error {
}

if a.podSelector.Select(pod) {
go a.streamLogs(ctx, pod)
go a.streamLogs(cancelCtx, pod)
}
}
}
Expand All @@ -111,6 +116,11 @@ func (a *LogAggregator) Start(ctx context.Context) error {
return nil
}

// Stop stops the logger.
func (a *LogAggregator) Stop() {
a.cancel()
}

func sinceSeconds(d time.Duration) int64 {
since := int64((d + 999*time.Millisecond).Truncate(1 * time.Second).Seconds())
if since != 0 {
Expand Down
74 changes: 37 additions & 37 deletions pkg/skaffold/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,26 +207,43 @@ func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*v1
// Create watcher and register artifacts to build current state of files.
changed := changes{}
onChange := func() error {
logger.Mute()
hasError := true

var err error
logger.Mute()
defer func() {
changed.reset()
color.Default.Fprintln(out, "Watching for changes...")
if !hasError {
logger.Unmute()
}
}()

switch {
case changed.needsReload:
err = ErrorConfigurationChanged
logger.Stop()
return ErrorConfigurationChanged
case len(changed.dirtyArtifacts) > 0:
err = r.buildAndDeploy(ctx, out, changed.dirtyArtifacts, imageList)
bRes, err := r.Build(ctx, out, r.Tagger, changed.dirtyArtifacts)
if err != nil {
logrus.Warnln("Skipping Deploy due to build error:", err)
return nil
}

r.updateBuiltImages(imageList, bRes)

if _, err = r.Deploy(ctx, out, r.builds); err != nil {
logrus.Warnln("Skipping Deploy due to error:", err)
return nil
}
case changed.needsRedeploy:
if _, err := r.Deploy(ctx, out, r.builds); err != nil {
logrus.Warnln("Skipping Deploy due to error:", err)
return nil
}
}

color.Default.Fprintln(out, "Watching for changes...")
changed.reset()
logger.Unmute()

return err
hasError = false
return nil
}

watcher := r.watchFactory()
Expand Down Expand Up @@ -264,8 +281,16 @@ func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*v1
}

// First run
if err := r.buildAndDeploy(ctx, out, artifacts, imageList); err != nil {
return nil, errors.Wrap(err, "first run")
bRes, err := r.Build(ctx, out, r.Tagger, artifacts)
if err != nil {
return nil, errors.Wrap(err, "exiting dev mode because the first build failed")
}

r.updateBuiltImages(imageList, bRes)

_, err = r.Deploy(ctx, out, r.builds)
if err != nil {
return nil, errors.Wrap(err, "exiting dev mode because the first deploy failed")
}

// Start logs
Expand All @@ -290,39 +315,14 @@ func (r *SkaffoldRunner) shouldWatch(artifact *v1alpha2.Artifact) bool {
return false
}

// buildAndDeploy builds a subset of the artifacts and deploys everything.
func (r *SkaffoldRunner) buildAndDeploy(ctx context.Context, out io.Writer, artifacts []*v1alpha2.Artifact, images *kubernetes.ImageList) error {
firstRun := r.builds == nil

bRes, err := r.Build(ctx, out, r.Tagger, artifacts)
if err != nil {
if firstRun {
return errors.Wrap(err, "exiting dev mode because the first build failed")
}

logrus.Warnln("Skipping Deploy due to build error:", err)
return nil
}

func (r *SkaffoldRunner) updateBuiltImages(images *kubernetes.ImageList, bRes []build.Artifact) {
// Update which images are logged.
for _, build := range bRes {
images.Add(build.Tag)
}

// Make sure all artifacts are redeployed. Not only those that were just rebuilt.
r.builds = mergeWithPreviousBuilds(bRes, r.builds)

_, err = r.Deploy(ctx, out, r.builds)
if err != nil {
if firstRun {
return errors.Wrap(err, "exiting dev mode because the first deploy failed")
}

logrus.Warnln("Skipping Deploy due to error:", err)
return nil
}

return nil
}

func mergeWithPreviousBuilds(builds, previous []build.Artifact) []build.Artifact {
Expand Down