add validation and migration for deprecated logentries driver

thaJeztah · thaJeztah · commit 3b1d9f1a26de · 2023-12-13T01:10:05.000+01:00
A validation step was added to prevent the daemon from considering "logentries"
as a dynamically loaded plugin, causing it to continue trying to load the plugin;

    WARN[2023-12-12T21:53:16.866857127Z] Unable to locate plugin: logentries, retrying in 1s
    WARN[2023-12-12T21:53:17.868296836Z] Unable to locate plugin: logentries, retrying in 2s
    WARN[2023-12-12T21:53:19.874259254Z] Unable to locate plugin: logentries, retrying in 4s
    WARN[2023-12-12T21:53:23.879869881Z] Unable to locate plugin: logentries, retrying in 8s

But would ultimately be returned as an error to the user:

    docker container create --name foo --log-driver=logentries nginx:alpine
    Error response from daemon: error looking up logging plugin logentries: plugin "logentries" not found

With the additional validation step, an error is returned immediately:

    docker container create --log-driver=logentries busybox
    Error response from daemon: the logentries logging driver has been deprecated and removed

A migration step was added on container restore. Containers using the
"logentries" logging driver are migrated to use the "local" logging driver:

    WARN[2023-12-12T22:38:53.108349297Z] migrated deprecated logentries logging driver  container=4c9309fedce75d807340ea1820cc78dc5c774d7bfcae09f3744a91b84ce6e4f7 error="&lt;nil&gt;"

As an alternative to the validation step, I also considered using a "stub"
deprecation driver, however this would not result in an error when creating
the container, and only produce an error when starting:

    docker container create --name foo --log-driver=logentries nginx:alpine
    4c9309fedce75d807340ea1820cc78dc5c774d7bfcae09f3744a91b84ce6e4f7

    docker start foo
    Error response from daemon: failed to create task for container: failed to initialize logging driver: the logentries logging driver has been deprecated and removed
    Error: failed to start containers: foo

For containers, this validation is added in the backend (daemon). For services,
this was not sufficient, as SwarmKit would try to schedule the task, which
caused a close loop;

    docker service create --log-driver=logentries --name foo nginx:alpine
    zo0lputagpzaua7cwga4lfmhp
    overall progress: 0 out of 1 tasks
    1/1: no suitable node (missing plugin on 1 node)
    Operation continuing in background.

    DEBU[2023-12-12T22:50:28.132732757Z] Calling GET /v1.43/tasks?filters=%7B%22_up-to-date%22%3A%7B%22true%22%3Atrue%7D%2C%22service%22%3A%7B%22zo0lputagpzaua7cwga4lfmhp%22%3Atrue%7D%7D
    DEBU[2023-12-12T22:50:28.137961549Z] Calling GET /v1.43/nodes
    DEBU[2023-12-12T22:50:28.340665007Z] Calling GET /v1.43/services/zo0lputagpzaua7cwga4lfmhp?insertDefaults=false
    DEBU[2023-12-12T22:50:28.343437632Z] Calling GET /v1.43/tasks?filters=%7B%22_up-to-date%22%3A%7B%22true%22%3Atrue%7D%2C%22service%22%3A%7B%22zo0lputagpzaua7cwga4lfmhp%22%3Atrue%7D%7D
    DEBU[2023-12-12T22:50:28.345201257Z] Calling GET /v1.43/nodes

So a validation was added in the service create and update endpoints;

    docker service create --log-driver=logentries --name foo nginx:alpine
    Error response from daemon: the logentries logging driver has been deprecated and removed

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
diff --git a/api/server/router/swarm/cluster_routes.go b/api/server/router/swarm/cluster_routes.go
@@ -209,6 +209,10 @@ func (sr *swarmRouter) createService(ctx context.Context, w http.ResponseWriter,
 	if err := httputils.ReadJSON(r, &service); err != nil {
 		return err
 	}
+	// TODO(thaJeztah): remove logentries check and migration code in release v26.0.0.
+	if service.TaskTemplate.LogDriver != nil && service.TaskTemplate.LogDriver.Name == "logentries" {
+		return errdefs.InvalidParameter(errors.New("the logentries logging driver has been deprecated and removed"))
+	}
 
 	// Get returns "" if the header does not exist
 	encodedAuth := r.Header.Get(registry.AuthHeader)
@@ -245,6 +249,10 @@ func (sr *swarmRouter) updateService(ctx context.Context, w http.ResponseWriter,
 	if err := httputils.ReadJSON(r, &service); err != nil {
 		return err
 	}
+	// TODO(thaJeztah): remove logentries check and migration code in release v26.0.0.
+	if service.TaskTemplate.LogDriver != nil && service.TaskTemplate.LogDriver.Name == "logentries" {
+		return errdefs.InvalidParameter(errors.New("the logentries logging driver has been deprecated and removed"))
+	}
 
 	rawVersion := r.URL.Query().Get("version")
 	version, err := strconv.ParseUint(rawVersion, 10, 64)
diff --git a/daemon/create.go b/daemon/create.go
@@ -62,6 +62,10 @@ func (daemon *Daemon) containerCreate(ctx context.Context, daemonCfg *configStor
 	if opts.params.Config == nil {
 		return containertypes.CreateResponse{}, errdefs.InvalidParameter(runconfig.ErrEmptyConfig)
 	}
+	// TODO(thaJeztah): remove logentries check and migration code in release v26.0.0.
+	if opts.params.HostConfig != nil && opts.params.HostConfig.LogConfig.Type == "logentries" {
+		return containertypes.CreateResponse{}, errdefs.InvalidParameter(fmt.Errorf("the logentries logging driver has been deprecated and removed"))
+	}
 
 	// Normalize some defaults. Doing this "ad-hoc" here for now, as there's
 	// only one field to migrate, but we should consider having a better
diff --git a/daemon/daemon.go b/daemon/daemon.go
@@ -41,6 +41,7 @@ import (
 	_ "github.com/docker/docker/daemon/graphdriver/register" // register graph drivers
 	"github.com/docker/docker/daemon/images"
 	dlogger "github.com/docker/docker/daemon/logger"
+	"github.com/docker/docker/daemon/logger/local"
 	"github.com/docker/docker/daemon/network"
 	"github.com/docker/docker/daemon/snapshotter"
 	"github.com/docker/docker/daemon/stats"
@@ -339,17 +340,31 @@ func (daemon *Daemon) restore(cfg *configStore) error {
 
 			baseLogger := log.G(context.TODO()).WithField("container", c.ID)
 
-			// Migrate containers that don't have the default ("no") restart-policy set.
-			// The RestartPolicy.Name field may be empty for containers that were
-			// created with versions before v25.0.0.
-			//
-			// We also need to set the MaximumRetryCount to 0, to prevent
-			// validation from failing (MaximumRetryCount is not allowed if
-			// no restart-policy ("none") is set).
-			if c.HostConfig != nil && c.HostConfig.RestartPolicy.Name == "" {
-				baseLogger.WithError(err).Debug("migrated restart-policy")
-				c.HostConfig.RestartPolicy.Name = containertypes.RestartPolicyDisabled
-				c.HostConfig.RestartPolicy.MaximumRetryCount = 0
+			if c.HostConfig != nil {
+				// Migrate containers that don't have the default ("no") restart-policy set.
+				// The RestartPolicy.Name field may be empty for containers that were
+				// created with versions before v25.0.0.
+				//
+				// We also need to set the MaximumRetryCount to 0, to prevent
+				// validation from failing (MaximumRetryCount is not allowed if
+				// no restart-policy ("none") is set).
+				if c.HostConfig.RestartPolicy.Name == "" {
+					baseLogger.Debug("migrated restart-policy")
+					c.HostConfig.RestartPolicy.Name = containertypes.RestartPolicyDisabled
+					c.HostConfig.RestartPolicy.MaximumRetryCount = 0
+				}
+
+				// Migrate containers that use the deprecated (and now non-functional)
+				// logentries driver. Update them to use the "local" logging driver
+				// instead.
+				//
+				// TODO(thaJeztah): remove logentries check and migration code in release v26.0.0.
+				if c.HostConfig.LogConfig.Type == "logentries" {
+					baseLogger.Warn("migrated deprecated logentries logging driver")
+					c.HostConfig.LogConfig = containertypes.LogConfig{
+						Type: local.Name,
+					}
+				}
 			}
 
 			if err := daemon.checkpointAndSave(c); err != nil {
