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.

Sign up for GitHub

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

fix(hash): recreate container on project config content change #11931

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+448 −73
Open

fix(hash): recreate container on project config content change #11931

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a84e449
fix(hash): recreate container on project config content change
idsulik Jun 23, 2024
0056225
Revert "fix(hash): recreate container on project config content change"
idsulik Jul 11, 2024
fe61eaf
feat: add separate hashes for service configs and secrets, add file f…
idsulik Jul 22, 2024
f97276d
fixes
idsulik Dec 15, 2024
4b60861
fix linters
idsulik Feb 17, 2025
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
Prev Previous commit
Next Next commit
feat: add separate hashes for service configs and secrets, add file f… 
…older support

Signed-off-by: Suleiman Dibirov <idsulik@gmail.com>
  • Loading branch information
@idsulik
idsulik committed Feb 24, 2025
commit fe61eaf8256b765e2dae160976f294cb5641c483
6 changes: 4 additions & 2 deletions pkg/api/labels.go
View file
Original file line number Diff line number Diff line change
@@ -31,8 +31,10 @@ const (
ServiceLabel = "com.docker.compose.service"
// ConfigHashLabel stores configuration hash for a compose service
ConfigHashLabel = "com.docker.compose.config-hash"
// ConfigHashDependenciesLabel stores configuration hash for a compose service dependencies
ConfigHashDependenciesLabel = "com.docker.compose.config-hash-dependencies"
// ServiceConfigsHash stores configuration hash for a compose service configs
ServiceConfigsHash = "com.docker.compose.service.configs-hash"
// ServiceSecretsHash stores configuration hash for a compose service secrets
ServiceSecretsHash = "com.docker.compose.service.secrets-hash"
// ContainerNumberLabel stores the container index of a replicated service
ContainerNumberLabel = "com.docker.compose.container-number"
// VolumeLabel allow to track resource related to a compose volume
14 changes: 10 additions & 4 deletions pkg/compose/convergence.go
View file
Original file line number Diff line number Diff line change
@@ -343,13 +343,19 @@ func (c *convergence) mustRecreate(project *types.Project,, expected types.Servi
return true, nil
}

serviceDependenciesHash, err := ServiceDependenciesHash(project, expected)
serviceConfigsHash, err := ServiceConfigsHash(project, expected)
if err != nil {
return false, err
}
configChanged := actual.Labels[api.ConfigHashLabel] != configHash
imageUpdated := actual.Labels[api.ImageDigestLabel] != expected.CustomLabels[api.ImageDigestLabel]
if configChanged || imageUpdated {

serviceSecretsHash, err := ServiceSecretsHash(project, expected)
if err != nil {
return false, err
}
serviceConfigsChanged := actual.Labels[api.ServiceConfigsHash] != serviceConfigsHash
serviceSecretsChanged := actual.Labels[api.ServiceSecretsHash] != serviceSecretsHash

if serviceConfigsChanged || serviceSecretsChanged {
return true, nil
}

10 changes: 8 additions & 2 deletions pkg/compose/create.go
View file
Original file line number Diff line number Diff line change
@@ -513,7 +513,12 @@ func (s *composeService) prepareLabels(labels types.Labels, project *types.Proje
return nil, err
}

serviceDependenciesHash, err := ServiceDependenciesHash(project, service)
serviceConfigsHash, err := ServiceConfigsHash(project, service)
if err != nil {
return nil, err
}

serviceSecretsHash, err := ServiceSecretsHash(project, service)
if err != nil {
return nil, err
}
@@ -523,7 +528,8 @@ func (s *composeService) prepareLabels(labels types.Labels, project *types.Proje
labels[api.ContainerNumberLabel] = strconv.Itoa(number)
}
labels[api.ConfigHashLabel] = serviceHash
labels[api.ConfigHashDependenciesLabel] = serviceDependenciesHash
labels[api.ServiceConfigsHash] = serviceConfigsHash
labels[api.ServiceSecretsHash] = serviceSecretsHash
labels[api.ContainerNumberLabel] = strconv.Itoa(number)

var dependencies []string
86 changes: 65 additions & 21 deletions pkg/compose/hash.go
View file
Original file line number Diff line number Diff line change
@@ -17,10 +17,13 @@
package compose

import (
"bytes"
"encoding/json"
"os"
"fmt"
"time"

"github.com/compose-spec/compose-go/v2/types"
"github.com/docker/compose/v2/pkg/utils"
"github.com/opencontainers/go-digest"
)

@@ -36,36 +39,77 @@ func ServiceHash(o types.ServiceConfig) (string, error) {
o.DependsOn = nil
o.Profiles = nil

bytes, err := json.Marshal(o)
data, err := json.Marshal(o)
if err != nil {
return "", err
}
return digest.SHA256.FromBytes(bytes).Encoded(), nil
return digest.SHA256.FromBytes(data).Encoded(), nil
}

// ServiceDependenciesHash computes the configuration hash for service dependencies.
func ServiceDependenciesHash(project *types.Project, o types.ServiceConfig) (string, error) {
bytes := make([]byte, 0)
for _, serviceConfig := range o.Configs {
projectConfig, ok := project.Configs[serviceConfig.Source]
if !ok {
continue
// ServiceConfigsHash computes the configuration hash for service configs.
func ServiceConfigsHash(project *types.Project, serviceConfig types.ServiceConfig) (string, error) {
data := make([]byte, 0)
for _, config := range serviceConfig.Configs {
file := project.Configs[config.Source]
b, err := createTarForConfig(project, types.FileReferenceConfig(config), types.FileObjectConfig(file))

if err != nil {
return "", err
}

data = append(data, b.Bytes()...)
}

return digest.SHA256.FromBytes(data).Encoded(), nil
Copy link
Contributor

@ndeloof ndeloof Oct 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer we have one label per config/secret mount, so it makes it easier to track|debug changes and container being recreated.
Also need to consider config can be mounted from docker host, i.e. file is not available for compose to compute hash, and then must be excluded from label / no label created. createTarForConfig could return ErrNotFound and we would ignore it for this specific usage

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean something like that:

com.docker.compose.service.configs-hash-{configName}={hash}
com.docker.compose.service.configs-hash-{serviceName}={hash}

?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed, or maybe, to follow the dot-notation style used for labels, com.docker.compose.service.configs.{configName}.hash

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that this will complicate the logic, first you need to generate a hash for each item separately, then you need to go through all labels whose names start with “com.docker.compose.service.configs.” to check if the hash has changed.

Copy link
Contributor

@ndeloof ndeloof Oct 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't look such a pain to me, as this would allow to trace reason we recreate a container, and make it easier to diagnose potential regressions (this sometimes happened :P)

for c := range service.Configs {
  hash := labels["com.docker.compose.configs."+c+".hash"]
  expected := ConfigHash(project.Configs[c]
  if hash := expected {
    log.Debug("container has to be recreated after config %s has been updated", c)
    return DIVERGED
  }
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ndeloof, but we don't have a config/service name that we can use to create the label name. I pushed changes to create a hash of the config/secret of each service so it'll be easy to figure out what service's config/secret caused the change

}

// ServiceSecretsHash computes the configuration hash for service secrets.
func ServiceSecretsHash(project *types.Project, serviceConfig types.ServiceConfig) (string, error) {
data := make([]byte, 0)
for _, secret := range serviceConfig.Secrets {
file := project.Secrets[secret.Source]
b, err := createTarForConfig(project, types.FileReferenceConfig(secret), types.FileObjectConfig(file))

if err != nil {
return "", err
}

if projectConfig.Content != "" {
bytes = append(bytes, []byte(projectConfig.Content)...)
} else if projectConfig.File != "" {
content, err := os.ReadFile(projectConfig.File)
if err != nil {
return "", err
}
bytes = append(bytes, content...)
} else if projectConfig.Environment != "" {
bytes = append(bytes, []byte(projectConfig.Environment)...)
data = append(data, b.Bytes()...)
}

return digest.SHA256.FromBytes(data).Encoded(), nil
}

func createTarForConfig(
project *types.Project,
serviceConfig types.FileReferenceConfig,
file types.FileObjectConfig,
) (*bytes.Buffer, error) {
// fixed time to ensure the tarball is deterministic
modTime := time.Unix(0, 0)

if serviceConfig.Target == "" {
serviceConfig.Target = "/" + serviceConfig.Source
}

switch {
case file.Content != "":
return bytes.NewBuffer([]byte(file.Content)), nil
case file.Environment != "":
env, ok := project.Environment[file.Environment]
if !ok {
return nil, fmt.Errorf(
"environment variable %q required by file %q is not set",
file.Environment,
file.Name,
)
}
return bytes.NewBuffer([]byte(env)), nil
case file.File != "":
return utils.CreateTarByPath(file.File, modTime)
}

return digest.SHA256.FromBytes(bytes).Encoded(), nil
return nil, fmt.Errorf("config %q is empty", file.Name)
}

// NetworkHash computes the configuration hash for a network.
107 changes: 90 additions & 17 deletions pkg/compose/hash_test.go
View file
Original file line number Diff line number Diff line change
@@ -39,56 +39,124 @@ func TestServiceHashWithIgnorableValues(t *testing.T) {
assert.Equal(t, hash1, hash2)
}

func TestServiceDependenciesHashWithoutChangesContent(t *testing.T) {
hash1, err := ServiceDependenciesHash(projectConfig("myConfigSource", "a", "", ""), serviceConfig("myContext1", "always", 1))
func TestServiceConfigsHashWithoutChangesContent(t *testing.T) {
hash1, err := ServiceConfigsHash(projectWithConfigs("a", "", ""), serviceConfig("myContext1", "always", 1))
assert.NilError(t, err)
hash2, err := ServiceDependenciesHash(projectConfig("myConfigSource", "a", "", ""), serviceConfig("myContext2", "never", 2))
hash2, err := ServiceConfigsHash(projectWithConfigs("a", "", ""), serviceConfig("myContext2", "never", 2))
assert.NilError(t, err)
assert.Assert(t, hash1 == hash2)
}

func TestServiceDependenciesHashWithChangedConfigContent(t *testing.T) {
hash1, err := ServiceDependenciesHash(projectConfig("myConfigSource", "a", "", ""), serviceConfig("myContext1", "always", 1))
func TestServiceConfigsHashWithChangedConfigContent(t *testing.T) {
hash1, err := ServiceConfigsHash(projectWithConfigs("a", "", ""), serviceConfig("myContext1", "always", 1))
assert.NilError(t, err)
hash2, err := ServiceDependenciesHash(projectConfig("myConfigSource", "b", "", ""), serviceConfig("myContext2", "never", 2))
hash2, err := ServiceConfigsHash(projectWithConfigs("b", "", ""), serviceConfig("myContext2", "never", 2))
assert.NilError(t, err)
assert.Assert(t, hash1 != hash2)
}

func TestServiceDependenciesHashWithChangedConfigEnvironment(t *testing.T) {
hash1, err := ServiceDependenciesHash(projectConfig("myConfigSource", "", "a", ""), serviceConfig("myContext1", "always", 1))
func TestServiceConfigsHashWithChangedConfigEnvironment(t *testing.T) {
hash1, err := ServiceConfigsHash(projectWithConfigs("", "a", ""), serviceConfig("myContext1", "always", 1))
assert.NilError(t, err)
hash2, err := ServiceDependenciesHash(projectConfig("myConfigSource", "", "b", ""), serviceConfig("myContext2", "never", 2))
hash2, err := ServiceConfigsHash(projectWithConfigs("", "b", ""), serviceConfig("myContext2", "never", 2))
assert.NilError(t, err)
assert.Assert(t, hash1 != hash2)
}

func TestServiceDependenciesHashWithChangedConfigFile(t *testing.T) {
hash1, err := ServiceDependenciesHash(
projectConfig("myConfigSource", "", "", "./testdata/config1.txt"),
func TestServiceConfigsHashWithChangedConfigFile(t *testing.T) {
hash1, err := ServiceConfigsHash(
projectWithConfigs("", "", "./testdata/config1.txt"),
serviceConfig("myContext1", "always", 1),
)
assert.NilError(t, err)
hash2, err := ServiceDependenciesHash(
projectConfig("myConfigSource", "", "", "./testdata/config2.txt"),
hash2, err := ServiceConfigsHash(
projectWithConfigs("", "", "./testdata/config2.txt"),
serviceConfig("myContext2", "never", 2),
)
assert.NilError(t, err)
assert.Assert(t, hash1 != hash2)
}

func projectConfig(configName, configContent, configEnvironment, configFile string) *types.Project {
func TestServiceSecretsHashWithoutChangesContent(t *testing.T) {
hash1, err := ServiceSecretsHash(projectWithSecrets("a", "", ""), serviceConfig("myContext1", "always", 1))
assert.NilError(t, err)
hash2, err := ServiceSecretsHash(projectWithSecrets("a", "", ""), serviceConfig("myContext2", "never", 2))
assert.NilError(t, err)
assert.Assert(t, hash1 == hash2)
}

func TestServiceSecretsHashWithChangedSecretContent(t *testing.T) {
hash1, err := ServiceSecretsHash(projectWithSecrets("a", "", ""), serviceConfig("myContext1", "always", 1))
assert.NilError(t, err)
hash2, err := ServiceSecretsHash(projectWithSecrets("b", "", ""), serviceConfig("myContext2", "never", 2))
assert.NilError(t, err)
assert.Assert(t, hash1 != hash2)
}

func TestServiceSecretsHashWithChangedSecretEnvironment(t *testing.T) {
hash1, err := ServiceSecretsHash(projectWithSecrets("", "a", ""), serviceConfig("myContext1", "always", 1))
assert.NilError(t, err)
hash2, err := ServiceSecretsHash(projectWithSecrets("", "b", ""), serviceConfig("myContext2", "never", 2))
assert.NilError(t, err)
assert.Assert(t, hash1 != hash2)
}

func TestServiceSecretsHashWithChangedSecretFile(t *testing.T) {
hash1, err := ServiceSecretsHash(
projectWithSecrets("", "", "./testdata/config1.txt"),
serviceConfig("myContext1", "always", 1),
)
assert.NilError(t, err)
hash2, err := ServiceSecretsHash(
projectWithSecrets("", "", "./testdata/config2.txt"),
serviceConfig("myContext2", "never", 2),
)
assert.NilError(t, err)
assert.Assert(t, hash1 != hash2)
}

func projectWithConfigs(configContent, configEnvironmentValue, configFile string) *types.Project {
envName := "myEnv"

if configEnvironmentValue == "" {
envName = ""
}

return &types.Project{
Environment: types.Mapping{
envName: configEnvironmentValue,
},
Configs: types.Configs{
configName: types.ConfigObjConfig{
"myConfigSource": types.ConfigObjConfig{
Content: configContent,
Environment: configEnvironment,
Environment: envName,
File: configFile,
},
},
}
}

func projectWithSecrets(secretContent, secretEnvironmentValue, secretFile string) *types.Project {
envName := "myEnv"

if secretEnvironmentValue == "" {
envName = ""
}

return &types.Project{
Environment: types.Mapping{
envName: secretEnvironmentValue,
},
Secrets: types.Secrets{
"mySecretSource": types.SecretConfig{
Content: secretContent,
Environment: envName,
File: secretFile,
},
},
}
}

func serviceConfig(buildContext, pullPolicy string, replicas int) types.ServiceConfig {
return types.ServiceConfig{
Build: &types.BuildConfig{
@@ -106,5 +174,10 @@ func serviceConfig(buildContext, pullPolicy string, replicas int) types.ServiceC
Source: "myConfigSource",
},
},
Secrets: []types.ServiceSecretConfig{
{
Source: "mySecretSource",
},
},
}
}
Loading
Oops, something went wrong.