Consider Labels in volume hash

Signed-off-by: Joana Hrotko <[email protected]>

pkg/compose/convergence.go

@@ -115,13 +115,13 @@ func (c *convergence) apply(ctx context.Context, project *types.Project, options
 	})
 }
 
-func (c *convergence) getServiceVolumes(service types.ServiceConfig) map[string]volumetypes.Volume {
-	volumes := make(map[string]volumetypes.Volume, len(service.Volumes))
+func (c *convergence) getServiceVolumes(service types.ServiceConfig) ComposeVolumes {
+	volumes := make(ComposeVolumes, len(service.Volumes))
 
 	for _, v := range service.Volumes {
 		observedVolume := c.getObservedVolumeState(v.Source)
 		if observedVolume != nil {
-			volumes[v.Source] = *observedVolume
+			volumes[v.Source] = observedVolume
 		}
 	}
 	return volumes
@@ -337,7 +337,7 @@ func (c *convergence) resolveSharedNamespaces(service *types.ServiceConfig) erro
 	return nil
 }
 
-func mustRecreate(project *types.Project, expected types.ServiceConfig, actual moby.Container, policy string, volumes map[string]volumetypes.Volume) (bool, error) {
+func mustRecreate(project *types.Project, expected types.ServiceConfig, actual moby.Container, policy string, volumes ComposeVolumes) (bool, error) {
 	if policy == api.RecreateNever {
 		return false, nil
 	}
@@ -359,22 +359,27 @@ func mustRecreate(project *types.Project, expected types.ServiceConfig, actual m
 	return configChanged || imageUpdated || volumesUpdated, nil
 }
 
-func compareVolumeMount(project *types.Project, service types.ServiceConfig, actualVolumes map[string]volumetypes.Volume) (bool, error) {
+func compareVolumeMount(project *types.Project, service types.ServiceConfig, actualVolumes ComposeVolumes) (bool, error) {
 	// we should recreate the container if we need to recreate the volume
 	var needsUpdate = false
 	for _, volume := range service.Volumes {
 		name := volume.Source
-		// we need to check hash volumes (from inspect that should be inside the convergence) labels
 		if v, ok := actualVolumes[name]; ok {
-			// get volume hash from inspected volumes
-			volumeConfig := project.Volumes[name]
+			// we need to check volume hashes volumes
+			volumeConfig, ok := project.Volumes[name]
+			if !ok {
+				continue
+			}
+			previousHash, ok := v.Labels[api.VolumeConfigHashLabel]
+			if !ok {
+				continue
+			}
 
 			// get current hash for service volume
 			currentHash, err := VolumeHash(volumeConfig)
 			if err != nil {
 				return false, err
 			}
-			previousHash := v.Labels[api.VolumeConfigHashLabel]
 			needsUpdate = needsUpdate || currentHash != previousHash
 		}
 	}

pkg/compose/create.go

@@ -65,6 +65,8 @@ type createConfigs struct {
 	Links     []string
 }
 
+type ComposeVolumes map[string]*volumetypes.Volume
+
 func (s *composeService) Create(ctx context.Context, project *types.Project, createOpts api.CreateOptions) error {
 	return progress.RunWithTitle(ctx, func(ctx context.Context) error {
 		return s.create(ctx, project, createOpts)
@@ -122,7 +124,7 @@ func (s *composeService) create(ctx context.Context, project *types.Project, opt
 	return newConvergence(options.Services, observedState, s, observedVolumesState).apply(ctx, project, options)
 }
 
-func (s *composeService) getVolumes(ctx context.Context, projectName string) (map[string]*volumetypes.Volume, error) {
+func (s *composeService) getVolumes(ctx context.Context, projectName string) (ComposeVolumes, error) {
 	volumeList, err := s.apiClient().VolumeList(ctx, volumetypes.ListOptions{
 		Filters: filters.NewArgs(projectFilter(projectName)),
 	})
@@ -133,7 +135,7 @@ func (s *composeService) getVolumes(ctx context.Context, projectName string) (ma
 		return nil, nil
 	}
 
-	volumes := make(map[string]*volumetypes.Volume, len(volumeList.Volumes))
+	volumes := make(ComposeVolumes, len(volumeList.Volumes))
 	for _, v := range volumeList.Volumes {
 		name := v.Labels[api.VolumeLabel]
 		volumes[name] = v
@@ -161,34 +163,31 @@ func (s *composeService) ensureNetworks(ctx context.Context, networks types.Netw
 	return nil
 }
 
-func (s *composeService) ensureProjectVolumes(ctx context.Context, project *types.Project, currentState map[string]*volumetypes.Volume, recreateVolumes bool) error {
+func (s *composeService) ensureProjectVolumes(ctx context.Context, project *types.Project, currentState ComposeVolumes, recreateVolumes bool) error {
 	for k, volume := range project.Volumes {
 		volume.Labels = volume.Labels.Add(api.VolumeLabel, k)
 		volume.Labels = volume.Labels.Add(api.ProjectLabel, project.Name)
 		volume.Labels = volume.Labels.Add(api.VersionLabel, api.ComposeVersion)
-
 		volumeConfigHash, err := VolumeHash(volume)
-		volume.Labels = volume.Labels.Add(api.VolumeConfigHashLabel, volumeConfigHash)
 		if err != nil {
 			return err
 		}
+		volume.Labels = volume.Labels.Add(api.VolumeConfigHashLabel, volumeConfigHash)
+
 		err = s.ensureVolume(ctx, volume, project.Name)
 		if err != nil {
 			return err
 		}
 
-		if recreateVolumes {
-			actualVolume, ok := currentState[k]
-			if !ok {
-				return nil
-			}
+		actualVolume, exists := currentState[k]
+		if recreateVolumes && exists {
 			// Check if it's necessary to recreate the volume by comparing
 			// with the current volume config
-			ok, err = shouldUpdateVolume(volume, actualVolume)
+			shouldRecreateVolume, err := shouldUpdateVolume(volume, actualVolume)
 			if err != nil {
 				return err
 			}
-			if !ok {
+			if !shouldRecreateVolume {
 				continue
 			}
 			logrus.Warnf("Warning: Running with --recreate-volumes will delete data for volume %s", k)
@@ -1482,6 +1481,7 @@ func (s *composeService) ensureVolume(ctx context.Context, volume types.VolumeCo
 		if volume.External {
 			return fmt.Errorf("external volume %q not found", volume.Name)
 		}
+		fmt.Println("creating volume")
 		err := s.createVolume(ctx, volume)
 		return err
 	}

pkg/compose/hash.go

@@ -20,6 +20,7 @@ import (
 	"encoding/json"
 
 	"github.com/compose-spec/compose-go/v2/types"
+	"github.com/docker/compose/v2/pkg/api"
 	"github.com/opencontainers/go-digest"
 )
 
@@ -41,13 +42,29 @@ func ServiceHash(o types.ServiceConfig) (string, error) {
 	return digest.SHA256.FromBytes(bytes).Encoded(), nil
 }
 
+func DeepCopy(src *types.VolumeConfig) (types.VolumeConfig, error) {
+	var dst types.VolumeConfig
+	data, err := json.Marshal(src)
+	if err != nil {
+		return types.VolumeConfig{}, err
+	}
+	err = json.Unmarshal(data, &dst)
+	return dst, err
+}
+
 // From a top-level Volume Configuration, creates a unique hash ignoring
 // External
-func VolumeHash(o types.VolumeConfig) (string, error) {
+func VolumeHash(v types.VolumeConfig) (string, error) {
+	o, err := DeepCopy(&v)
+	if err != nil {
+		return "", err
+	}
+
 	if o.Driver == "" { // (TODO: jhrotko) This probably should be fixed in compose-go
 		o.Driver = "local"
 	}
 	o.External = false // (TODO: jhrotko) the name can change. Need to think about this case
+	delete(o.Labels, api.VolumeConfigHashLabel)
 
 	bytes, err := json.Marshal(o)
 	if err != nil {