Annotate "stack" commands to be "swarm" and "kubernetes"

[thaJeztah]

The stack commands require either "swarm" or "kubernetes"

- Description for the changelog

ping @dnephin @vdemeester @MistyHacks

[thaJeztah]

I wasn't sure if I should add "kubernetes" for the top-level docker deploy; it's still marked "experimental" (and I think we should consider removing it)

[vdemeester]

Yes, I think we should remove the top level one (and thus we don't need to annotate it)

[codecov-io]

Codecov Report

Merging #804 into master will decrease coverage by 0.01%.
The diff coverage is 19.04%.

@@            Coverage Diff             @@
##           master     #804      +/-   ##
==========================================
- Coverage   52.96%   52.95%   -0.02%     
==========================================
  Files         244      244              
  Lines       15828    15839      +11     
==========================================
+ Hits         8383     8387       +4     
- Misses       6891     6898       +7     
  Partials      554      554

[mdlinville]

LGTM, needs to be cherry-picked definitely.

[vdemeester]

LGTM 🐒

[vdemeester]

Yes, I think we should remove the top level one (and thus we don't need to annotate it)

[thaJeztah]

ping @dnephin okay to merge?

[dnephin]

there is a test failure, let me try to rerun

[dnephin]

@thaJeztah the e2e test seems to be failing consistently. Can you take a look? I think it's a real failure, doesn't look like a flake.

[dnephin]

I think the logic for disabling kubernetes doesn't account for swarm also being set ?

[thaJeztah]

oh, interesting one; I'd have to check where/how it's used there 😓

[vdemeester]

@dnephin @thaJeztah right.. the code that checks for swarm/kubernetes is pretty naive and doesn't handle something that would support both (i.e. it make the assumption that it would have none of both…). That needs to change 😅

[vdemeester]

diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go
index 74d8447f..cd5be656 100644
--- a/cmd/docker/docker.go
+++ b/cmd/docker/docker.go
@@ -277,10 +277,12 @@ func areFlagsSupported(cmd *cobra.Command, details versionDetails) error {
                        if _, ok := f.Annotations["experimentalCLI"]; ok && !hasExperimentalCLI {
                                errs = append(errs, fmt.Sprintf("\"--%s\" is only supported when experimental cli features are enabled", f.Name))
                        }
-                       if _, ok := f.Annotations["kubernetes"]; ok && !hasKubernetes {
+                       _, isKubernetesAnnotated := f.Annotations["kubernetes"]
+                       _, isSwarmAnnotated := f.Annotations["swarm"]
+                       if isKubernetesAnnotated && !isSwarmAnnotated && !hasKubernetes {
                                errs = append(errs, fmt.Sprintf("\"--%s\" is only supported on a Docker cli with kubernetes features enabled", f.Name))
                        }
-                       if _, ok := f.Annotations["swarm"]; ok && hasKubernetes {
+                       if isSwarmAnnotated && !isKubernetesAnnotated && hasKubernetes {
                                errs = append(errs, fmt.Sprintf("\"--%s\" is only supported on a Docker cli with swarm features enabled", f.Name))
                        }
                }
@@ -309,10 +311,12 @@ func areSubcommandsSupported(cmd *cobra.Command, details versionDetails) error {
                if _, ok := curr.Annotations["experimentalCLI"]; ok && !hasExperimentalCLI {
                        return fmt.Errorf("%s is only supported when experimental cli features are enabled", cmd.CommandPath())
                }
-               if _, ok := curr.Annotations["kubernetes"]; ok && !hasKubernetes {
+               _, isKubernetesAnnotated := curr.Annotations["kubernetes"]
+               _, isSwarmAnnotated := curr.Annotations["swarm"]
+               if isKubernetesAnnotated && !isSwarmAnnotated && !hasKubernetes {
                        return fmt.Errorf("%s is only supported on a Docker cli with kubernetes features enabled", cmd.CommandPath())
                }
-               if _, ok := curr.Annotations["swarm"]; ok && hasKubernetes {
+               if isSwarmAnnotated && !isKubernetesAnnotated && hasKubernetes {
                        return fmt.Errorf("%s is only supported on a Docker cli with swarm features enabled", cmd.CommandPath())
                }
        }

Something like the above should fix it 👼

[thaJeztah]

Thanks! Added that change 👍

[thaJeztah]

oh, we may need something similar for areSubcommandsSupported(), let me check if that's indeed the case

[thaJeztah]

We should rewrite that logic a bit; it's quite confusing because !hasKubernetes means no orchestrator set means use default means swarm; we can do so in a follow up, as it may require some changes in other locations

[vdemeester]

ping @dnephin 👼

[dnephin]

LGTM

This will need some cleanup if we ever add a third option, but hopefully that wont happen for a while.