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

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
4 changes: 4 additions & 0 deletions pkg/api/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ const (
ServiceLabel = "com.docker.compose.service"
// ConfigHashLabel stores configuration hash for a compose service
ConfigHashLabel = "com.docker.compose.config-hash"
// 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
Expand Down
36 changes: 28 additions & 8 deletions pkg/compose/convergence.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,11 @@ func (c *convergence) ensureService(ctx context.Context, project *types.Project,

sort.Slice(containers, func(i, j int) bool {
// select obsolete containers first, so they get removed as we scale down
if obsolete, _ := mustRecreate(service, containers[i], recreate); obsolete {
if obsolete, _ := mustRecreate(project, service, containers[i], recreate); obsolete {
// i is obsolete, so must be first in the list
return true
}
if obsolete, _ := mustRecreate(service, containers[j], recreate); obsolete {
if obsolete, _ := mustRecreate(project, service, containers[j], recreate); obsolete {
// j is obsolete, so must be first in the list
return false
}
Expand All @@ -154,7 +154,7 @@ func (c *convergence) ensureService(ctx context.Context, project *types.Project,
continue
}

mustRecreate, err := mustRecreate(service, container, recreate)
mustRecreate, err := mustRecreate(project, service, container, recreate)
if err != nil {
return err
}
Expand Down Expand Up @@ -312,20 +312,40 @@ func (c *convergence) resolveSharedNamespaces(service *types.ServiceConfig) erro
return nil
}

func mustRecreate(expected types.ServiceConfig, actual moby.Container, policy string) (bool, error) {
func mustRecreate(project *types.Project, expected types.ServiceConfig, actual moby.Container, policy string) (bool, error) {
if policy == api.RecreateNever {
return false, nil
}
if policy == api.RecreateForce {
return true, nil
}
configHash, err := ServiceHash(expected)
serviceHash, err := ServiceHash(expected)
if err != nil {
return false, err
}
configChanged := actual.Labels[api.ConfigHashLabel] != configHash
imageUpdated := actual.Labels[api.ImageDigestLabel] != expected.CustomLabels[api.ImageDigestLabel]
return configChanged || imageUpdated, nil

if actual.Labels[api.ConfigHashLabel] != serviceHash {
return true, nil
}

if actual.Labels[api.ImageDigestLabel] != expected.CustomLabels[api.ImageDigestLabel] {
return true, nil
}

serviceConfigsHash, err := ServiceConfigsHash(project, expected)
if err != nil {
return false, err
}

serviceSecretsHash, err := ServiceSecretsHash(project, expected)
if err != nil {
return false, err
}

serviceConfigsChanged := actual.Labels[api.ServiceConfigsHash] != serviceConfigsHash
serviceSecretsChanged := actual.Labels[api.ServiceSecretsHash] != serviceSecretsHash

return serviceConfigsChanged || serviceSecretsChanged, nil
}

func getContainerName(projectName string, service types.ServiceConfig, number int) string {
Expand Down
20 changes: 16 additions & 4 deletions pkg/compose/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func (s *composeService) getCreateConfigs(ctx context.Context,
inherit *moby.Container,
opts createOptions,
) (createConfigs, error) {
labels, err := s.prepareLabels(opts.Labels, service, number)
labels, err := s.prepareLabels(opts.Labels, p, service, number)
if err != nil {
return createConfigs{}, err
}
Expand Down Expand Up @@ -498,13 +498,25 @@ func parseSecurityOpts(p *types.Project, securityOpts []string) ([]string, bool,
return parsed, unconfined, nil
}

func (s *composeService) prepareLabels(labels types.Labels, service types.ServiceConfig, number int) (map[string]string, error) {
hash, err := ServiceHash(service)
func (s *composeService) prepareLabels(labels types.Labels, project *types.Project, service types.ServiceConfig, number int) (map[string]string, error) {
serviceHash, err := ServiceHash(service)
if err != nil {
return nil, err
}
labels[api.ConfigHashLabel] = hash

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

serviceSecretsHash, err := ServiceSecretsHash(project, service)
if err != nil {
return nil, err
}

labels[api.ConfigHashLabel] = serviceHash
labels[api.ServiceConfigsHash] = serviceConfigsHash
labels[api.ServiceSecretsHash] = serviceSecretsHash
labels[api.ContainerNumberLabel] = strconv.Itoa(number)

var dependencies []string
Expand Down
125 changes: 125 additions & 0 deletions pkg/compose/hash.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,15 @@
package compose

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

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

Expand All @@ -40,3 +46,122 @@ func ServiceHash(o types.ServiceConfig) (string, error) {
}
return digest.SHA256.FromBytes(bytes).Encoded(), nil
}

// ServiceConfigsHash computes the configuration hash for service configs.
func ServiceConfigsHash(project *types.Project, serviceConfig types.ServiceConfig) (string, error) {
bytes := 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
}

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

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

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

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

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

return digest.SHA256.FromBytes(bytes).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)
content := make([]byte, 0)

if file.Content != "" {
content = []byte(file.Content)
} else if 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,
)
}
content = []byte(env)
} else if file.File != "" {
var err error
content, err = readPathContent(file.File)

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

if len(content) == 0 {
return nil, fmt.Errorf("config %q is empty", file.Name)
}

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

b, err := utils.CreateTar(content, serviceConfig, modTime)
if err != nil {
return nil, err
}

return &b, nil
}

func readPathContent(path string) ([]byte, error) {
content := make([]byte, 0)

// Check if the path is a directory
info, err := os.Stat(path)
if err != nil {
return nil, fmt.Errorf("error accessing path %q: %v", path, err)
}

if info.IsDir() {
// If it's a directory, read all files and concatenate their contents
err = filepath.Walk(path, func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if !info.IsDir() {
fileContent, err := os.ReadFile(path)
Copy link
Contributor

Choose a reason for hiding this comment

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

this will not detect file being rename. Maybe better to create a tar stream from file/dir content

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ndeloof it creates a tar from the content here pkg/compose/hash.go:122.

b, err := utils.CreateTar(content, serviceConfig, modTime)

if err != nil {
return err
}
content = append(content, fileContent...)
}
return nil
})
if err != nil {
return nil, fmt.Errorf("error reading directory %q: %v", path, err)
}
} else {
// If it's a file, read its content
fileContent, err := os.ReadFile(path)
if err != nil {
return nil, fmt.Errorf("error reading file %q: %v", path, err)
}
content = fileContent
}

return content, nil
}
Loading