Work with change to content-addressable IDs

- Strip the leading "sha256:" from content-addressable images to match
  the displayed IDs.
- Only warn if an image is pulled where the image ID doesn't match the
  recorded VersionID. Move warning to the code that starts the service.
- Don't stop containers when the image ID doesn't match. If the config
  version matches, it's the right container. If the image doesn't match,
  we'll still get warnings when checking for the latest image, and can
  fix by redeploying a new config.
- Don't InspectImage before pulling, since PullImage will do the same.
- Don't pull every 10s, since the service check will pull again.
- Warn when a contaner dies and is automatically restarted.
- Periodically warn when a container's image doesn't match the config.

Author: James Bardin

cmd/commander/main.go

@@ -107,27 +107,13 @@ func pullImageAsync(appCfg config.App, errChan chan error) {
 }
 
 func pullImage(appCfg config.App) (*docker.Image, error) {
-
-	image, err := serviceRuntime.InspectImage(appCfg.Version())
-	if image != nil && image.ID == appCfg.VersionID() || appCfg.VersionID() == "" {
-		return image, nil
-	}
-
-	log.Printf("Pulling %s version %s\n", appCfg.Name(), appCfg.Version())
-	image, err = serviceRuntime.PullImage(appCfg.Version(),
-		appCfg.VersionID())
+	image, err := serviceRuntime.PullImage(appCfg.Version(), appCfg.VersionID())
 	if image == nil || err != nil {
-		log.Errorf("ERROR: Could not pull image %s: %s",
-			appCfg.Version(), err)
+		log.Errorf("ERROR: Could not pull image %s: %s", appCfg.Version(), err)
 		return nil, err
 	}
 
-	if image.ID != appCfg.VersionID() && len(appCfg.VersionID()) > 12 {
-		log.Errorf("WARNING: Pulled image for %s does not match expected ID. Expected: %s: Got: %s",
-			appCfg.Version(), image.ID[0:12], appCfg.VersionID()[0:12])
-	}
-
-	log.Printf("Pulled %s\n", appCfg.Version())
+	log.Printf("Pulled %s version %s\n", appCfg.Name(), appCfg.Version())
 	return image, nil
 }
 
@@ -173,11 +159,13 @@ func startService(appCfg config.App, logStatus bool) {
 		}
 	}
 
-	err = serviceRuntime.StopAllButCurrentVersion(appCfg)
+	err = serviceRuntime.StopOldVersion(appCfg, -1)
 	if err != nil {
 		log.Errorf("ERROR: Could not stop old containers: %s", err)
 	}
 
+	// check the image version, and log any inconsistencies
+	inspectImage(appCfg)
 }
 
 func heartbeatHost() {
@@ -217,6 +205,21 @@ func appAssigned(app string) (bool, error) {
 	return true, nil
 }
 
+// inspectImage checks that the running image matches the config.
+// We only use this to print warnings, since we likely need to deploy a new
+// config version to fix the inconsistency.
+func inspectImage(appCfg config.App) {
+	image, err := serviceRuntime.InspectImage(appCfg.Version())
+	if err != nil {
+		log.Println("error inspecting image", appCfg.Version)
+		return
+	}
+
+	if utils.StripSHA(image.ID) != appCfg.VersionID() {
+		log.Printf("warning: %s image ID does not match config", appCfg.Name())
+	}
+}
+
 func restartContainers(app string, cmdChan chan string) {
 	defer wg.Done()
 	logOnce := true
@@ -314,14 +317,6 @@ func restartContainers(app string, cmdChan chan string) {
 				continue
 			}
 
-			_, err = pullImage(appCfg)
-			if err != nil {
-				if !loop {
-					return
-				}
-				log.Errorf("ERROR: Could not pull images: %s", err)
-				continue
-			}
 			startService(appCfg, logOnce)
 		}
 

commander/app.go

@@ -130,7 +130,7 @@ func AppDeploy(configStore *config.Store, serviceRuntime *runtime.ServiceRuntime
 	}
 
 	svcCfg.SetVersion(version)
-	svcCfg.SetVersionID(image.ID)
+	svcCfg.SetVersionID(utils.StripSHA(image.ID))
 
 	updated, err := configStore.UpdateApp(svcCfg, env)
 	if err != nil {

runtime/runtime.go

@@ -280,17 +280,21 @@ func (s *ServiceRuntime) StopOldVersion(appCfg config.App, limit int) error {
 
 		version := env["GALAXY_VERSION"]
 
-		imageDiffers := image.ID != appCfg.VersionID() && appCfg.VersionID() != ""
-		versionDiffers := version != strconv.FormatInt(appCfg.ID(), 10) && version != ""
+		if version == "" {
+			log.Printf("WARNING: %s missing GALAXY_VERSION", appCfg.ContainerName())
+		}
 
-		if imageDiffers || versionDiffers {
+		if version != strconv.FormatInt(appCfg.ID(), 10) && version != "" {
 			s.stopContainer(container)
 			stopped = stopped + 1
 		}
 	}
+
 	return nil
 }
 
+/*
+FIXME: using StopOldVersion(cfg, -1) for now
 func (s *ServiceRuntime) StopAllButCurrentVersion(appCfg config.App) error {
 	containers, err := s.ManagedContainers()
 	if err != nil {
@@ -328,6 +332,7 @@ func (s *ServiceRuntime) StopAllButCurrentVersion(appCfg config.App) error {
 	}
 	return nil
 }
+*/
 
 // TODO: these aren't called from anywhere. Are they useful?
 /*
@@ -607,16 +612,15 @@ func (s *ServiceRuntime) Start(env, pool string, appCfg config.App) (*docker.Con
 
 	img := appCfg.Version()
 
-	imgIdRef := appCfg.Version()
-	if appCfg.VersionID() != "" {
-		imgIdRef = appCfg.VersionID()
-	}
-	// see if we have the image locally
-	image, err := s.PullImage(img, imgIdRef)
+	image, err := s.PullImage(img, appCfg.VersionID())
 	if err != nil {
 		return nil, err
 	}
 
+	if utils.StripSHA(image.ID) != appCfg.VersionID() {
+		log.Warnf("warning: ID for image %s doesn't match configuration", img)
+	}
+
 	// setup env vars from etcd
 	var envVars []string
 	envVars = append(envVars, "ENV"+"="+env)
@@ -802,20 +806,23 @@ func findAuth(registry string) docker.AuthConfiguration {
 	return auths.Configs[defaultIndexServer]
 }
 
+// Pull a docker image.
+// If we have an image matching the tag, and the given id matches the current
+// image, don't fetch a new one from the registry.
 func (s *ServiceRuntime) PullImage(version, id string) (*docker.Image, error) {
 	image, err := s.InspectImage(version)
 
 	if err != nil && err != docker.ErrNoSuchImage {
 		return nil, err
 	}
 
-	if image != nil && image.ID == id {
+	if image != nil && utils.StripSHA(image.ID) == id {
 		return image, nil
 	}
 
 	registry, repository, tag := utils.SplitDockerImage(version)
 
-	// No, pull it down locally
+	// pull it down locally
 	pullOpts := docker.PullImageOptions{
 		Repository:   repository,
 		Tag:          tag,
@@ -837,11 +844,6 @@ func (s *ServiceRuntime) PullImage(version, id string) (*docker.Image, error) {
 		err = s.dockerClient.PullImage(pullOpts, dockerAuth)
 		if err != nil {
 
-			// Don't retry 404, they'll never succeed
-			if err.Error() == "HTTP code: 404" {
-				return image, nil
-			}
-
 			if retries > 3 {
 				return image, err
 			}
@@ -852,7 +854,6 @@ func (s *ServiceRuntime) PullImage(version, id string) (*docker.Image, error) {
 	}
 
 	return s.InspectImage(version)
-
 }
 
 func (s *ServiceRuntime) RegisterAll(env, pool, hostIP string) ([]*config.ServiceRegistration, error) {
@@ -967,6 +968,9 @@ func (s *ServiceRuntime) RegisterEvents(env, pool, hostIP string, listener chan
 
 						// if a container is restarting, don't continue re-registering the app
 						if container.State.Restarting {
+							if e.Status == "die" {
+								log.Println("WARN: restarting", container.Name)
+							}
 							continue
 						}
 

utils/utils.go

@@ -172,3 +172,8 @@ func ParseMemory(mem string) (int64, error) {
 	}
 	return i * multiplier, nil
 }
+
+// strip the leading sha256: from content adressable images
+func StripSHA(s string) string {
+	return strings.TrimPrefix(s, "sha256:")
+}