Deprecate Kubernetes stack support #3167

mat007 · 2021-06-29T12:59:35Z

- What I did

Added deprecated annotations to Kubernetes related flags.

- How I did it

- How to verify it

I believe it should end up as «deprecated» badges in the docs, but I’m not sure how to verify this.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

codecov-commenter · 2021-06-29T13:09:51Z

Codecov Report

Merging #3167 (a033cdf) into master (4a6fe51) will decrease coverage by 0.02%.
The diff coverage is 21.42%.

@@            Coverage Diff             @@
##           master    #3167      +/-   ##
==========================================
- Coverage   57.09%   57.06%   -0.03%     
==========================================
  Files         299      299              
  Lines       18728    18742      +14     
==========================================
+ Hits        10693    10696       +3     
- Misses       7165     7176      +11     
  Partials      870      870

mat007 · 2021-06-29T13:41:38Z

Oh also, what kind of time scale do we want between the deprecation notice and the actual code removal?
Is one release typically enough?

silvin-lubecki

You should also update the deprecated document:
https://github.com/docker/cli/blob/master/docs/deprecated.md

Ok looks like you already did it #3166 😅

mat007 · 2021-07-01T07:02:05Z

I added deprecation for Kubernetes in docker context as well, can you please take another look?

thaJeztah · 2021-07-01T12:30:45Z

cli/command/context/create.go

 flags.StringToStringVar(&opts.Docker, "docker", nil, "set the docker endpoint")
 flags.StringToStringVar(&opts.Kubernetes, "kubernetes", nil, "set the kubernetes endpoint")
+ flags.SetAnnotation("kubernetes", "kubernetes", nil)
+ flags.SetAnnotation("kubernetes", "deprecated", nil)


Curious; did you also try if flags.MarkDeprecated("kubernetes", "<deprecation message>") would do the trick?

Given: that also hides the flag, and prints a deprecation message when used, so it would have a slightly different result; https://github.com/spf13/pflag/blob/85dd5c8bc61cfa382fecd072378089d4e856579d/flag.go#L408-L422

// MarkDeprecated indicated that a flag is deprecated in your program. It will // continue to function but will not show up in help or usage messages. Using // this flag will also print the given usageMessage. func (f *FlagSet) MarkDeprecated(name string, usageMessage string) error { flag := f.Lookup(name) if flag == nil { return fmt.Errorf("flag %q does not exist", name) } if usageMessage == "" { return fmt.Errorf("deprecated message for flag %q must be set", name) } flag.Deprecated = usageMessage flag.Hidden = true return nil }

but will not show up in help or usage messages

Do we really want that?

I was expecting flags.SetAnnotation("kubernetes", "deprecated", nil) to actually end up marking the flag as deprecated in the docs, a bit like the "kubernetes" annotation (sorry it doesn’t help that "kubernetes" here it’s both a flag and an annotation 😬) in https://docs.docker.com/engine/reference/commandline/stack/#options
I think it has a good chance to work according to the code for the deprecated-badge and kubernetes at https://github.com/docker/docker.github.io/blob/master/_includes/cli.md#options

Do we really want that?

Gotcha.

Normally, yes (deprecate in "major" release X, and remove in "major release X+n"), but given that we're "cheating" a bit, and do this in a patch release, I it may indeed be a bit unexpected.

docs/yaml/yaml.go

Signed-off-by: Mathieu Champlon <mathieu.champlon@docker.com>

thaJeztah · 2021-07-02T14:32:44Z

Tried the generated yaml-docs;

make -f docker.Makefile yamldocs
mv docs/yaml/gen docs/yaml/gen-before

make -f docker.Makefile yamldocs
mv docs/yaml/gen docs/yaml/gen-after

git diff --no-index docs/yaml/gen-before docs/yaml/gen-after > changes.diff

diff below:

diff --git a/docs/yaml/gen-before/docker_context_create.yaml b/docs/yaml/gen-after/docker_context_create.yaml
index a3102090c..0a3a5b731 100644
--- a/docs/yaml/gen-before/docker_context_create.yaml
+++ b/docs/yaml/gen-after/docker_context_create.yaml
@@ -11,7 +11,7 @@ options:
   value_type: string
   description: |
     Default orchestrator for stack operations to use with this context (swarm|kubernetes|all)
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: false
@@ -45,10 +45,10 @@ options:
   value_type: stringToString
   default_value: '[]'
   description: set the kubernetes endpoint
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
-  kubernetes: false
+  kubernetes: true
   swarm: false
 examples: |-
   ### Create a context with a docker and kubernetes endpoint
diff --git a/docs/yaml/gen-before/docker_context_export.yaml b/docs/yaml/gen-after/docker_context_export.yaml
index 021fb0bd5..551815559 100644
--- a/docs/yaml/gen-before/docker_context_export.yaml
+++ b/docs/yaml/gen-after/docker_context_export.yaml
@@ -13,10 +13,10 @@ options:
   value_type: bool
   default_value: "false"
   description: Export as a kubeconfig file
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
-  kubernetes: false
+  kubernetes: true
   swarm: false
 deprecated: false
 experimental: false
diff --git a/docs/yaml/gen-before/docker_context_update.yaml b/docs/yaml/gen-after/docker_context_update.yaml
index d5980a68d..29b92a10b 100644
--- a/docs/yaml/gen-before/docker_context_update.yaml
+++ b/docs/yaml/gen-after/docker_context_update.yaml
@@ -11,7 +11,7 @@ options:
   value_type: string
   description: |
     Default orchestrator for stack operations to use with this context (swarm|kubernetes|all)
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: false
@@ -37,10 +37,10 @@ options:
   value_type: stringToString
   default_value: '[]'
   description: set the kubernetes endpoint
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
-  kubernetes: false
+  kubernetes: true
   swarm: false
 examples: |-
   ### Update an existing context
diff --git a/docs/yaml/gen-before/docker_stack.yaml b/docs/yaml/gen-after/docker_stack.yaml
index 3c378f8ac..dee0895bf 100644
--- a/docs/yaml/gen-before/docker_stack.yaml
+++ b/docs/yaml/gen-after/docker_stack.yaml
@@ -20,7 +20,7 @@ options:
 - option: kubeconfig
   value_type: string
   description: Kubernetes config file
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: true
@@ -28,7 +28,7 @@ options:
 - option: orchestrator
   value_type: string
   description: Orchestrator to use (swarm|kubernetes|all)
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: false
diff --git a/docs/yaml/gen-before/docker_stack_deploy.yaml b/docs/yaml/gen-after/docker_stack_deploy.yaml
index a42d84d32..7db53bb5f 100644
--- a/docs/yaml/gen-before/docker_stack_deploy.yaml
+++ b/docs/yaml/gen-after/docker_stack_deploy.yaml
@@ -28,7 +28,7 @@ options:
 - option: namespace
   value_type: string
   description: Kubernetes namespace to use
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: true
@@ -67,7 +67,7 @@ inherited_options:
 - option: kubeconfig
   value_type: string
   description: Kubernetes config file
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: true
@@ -75,7 +75,7 @@ inherited_options:
 - option: orchestrator
   value_type: string
   description: Orchestrator to use (swarm|kubernetes|all)
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: false
diff --git a/docs/yaml/gen-before/docker_stack_ls.yaml b/docs/yaml/gen-after/docker_stack_ls.yaml
index ea3c9db58..50d22c23b 100644
--- a/docs/yaml/gen-before/docker_stack_ls.yaml
+++ b/docs/yaml/gen-after/docker_stack_ls.yaml
@@ -18,7 +18,7 @@ options:
   value_type: bool
   default_value: "false"
   description: List stacks from all Kubernetes namespaces
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: true
@@ -35,7 +35,7 @@ options:
   value_type: stringSlice
   default_value: '[]'
   description: Kubernetes namespaces to use
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: true
@@ -44,7 +44,7 @@ inherited_options:
 - option: kubeconfig
   value_type: string
   description: Kubernetes config file
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: true
@@ -52,7 +52,7 @@ inherited_options:
 - option: orchestrator
   value_type: string
   description: Orchestrator to use (swarm|kubernetes|all)
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: false
diff --git a/docs/yaml/gen-before/docker_stack_ps.yaml b/docs/yaml/gen-after/docker_stack_ps.yaml
index 4176ec052..c04a79fd5 100644
--- a/docs/yaml/gen-before/docker_stack_ps.yaml
+++ b/docs/yaml/gen-after/docker_stack_ps.yaml
@@ -33,7 +33,7 @@ options:
 - option: namespace
   value_type: string
   description: Kubernetes namespace to use
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: true
@@ -70,7 +70,7 @@ inherited_options:
 - option: kubeconfig
   value_type: string
   description: Kubernetes config file
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: true
@@ -78,7 +78,7 @@ inherited_options:
 - option: orchestrator
   value_type: string
   description: Orchestrator to use (swarm|kubernetes|all)
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: false
diff --git a/docs/yaml/gen-before/docker_stack_rm.yaml b/docs/yaml/gen-after/docker_stack_rm.yaml
index ec951fe7d..5355ea86d 100644
--- a/docs/yaml/gen-before/docker_stack_rm.yaml
+++ b/docs/yaml/gen-after/docker_stack_rm.yaml
@@ -17,7 +17,7 @@ options:
 - option: namespace
   value_type: string
   description: Kubernetes namespace to use
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: true
@@ -26,7 +26,7 @@ inherited_options:
 - option: kubeconfig
   value_type: string
   description: Kubernetes config file
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: true
@@ -34,7 +34,7 @@ inherited_options:
 - option: orchestrator
   value_type: string
   description: Orchestrator to use (swarm|kubernetes|all)
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: false
diff --git a/docs/yaml/gen-before/docker_stack_services.yaml b/docs/yaml/gen-after/docker_stack_services.yaml
index 083da92e5..c34f83e94 100644
--- a/docs/yaml/gen-before/docker_stack_services.yaml
+++ b/docs/yaml/gen-after/docker_stack_services.yaml
@@ -33,7 +33,7 @@ options:
 - option: namespace
   value_type: string
   description: Kubernetes namespace to use
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: true
@@ -52,7 +52,7 @@ inherited_options:
 - option: kubeconfig
   value_type: string
   description: Kubernetes config file
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: true
@@ -60,7 +60,7 @@ inherited_options:
 - option: orchestrator
   value_type: string
   description: Orchestrator to use (swarm|kubernetes|all)
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: false
diff --git a/docs/yaml/gen-before/docker_version.yaml b/docs/yaml/gen-after/docker_version.yaml
index 90e72519f..227340849 100644
--- a/docs/yaml/gen-before/docker_version.yaml
+++ b/docs/yaml/gen-after/docker_version.yaml
@@ -22,7 +22,7 @@ options:
 - option: kubeconfig
   value_type: string
   description: Kubernetes config file
-  deprecated: false
+  deprecated: true
   experimental: false
   experimentalcli: false
   kubernetes: true

thaJeztah

LGTM, thanks!

mat007 requested review from silvin-lubecki and thaJeztah as code owners June 29, 2021 12:59

silvin-lubecki requested changes Jun 30, 2021

View reviewed changes

silvin-lubecki approved these changes Jun 30, 2021

View reviewed changes

mat007 requested a review from silvin-lubecki July 1, 2021 07:02

silvin-lubecki approved these changes Jul 1, 2021

View reviewed changes

thaJeztah reviewed Jul 1, 2021

View reviewed changes

mat007 added 2 commits July 1, 2021 18:39

Deprecate Kubernetes stack support

c05f0f5

Signed-off-by: Mathieu Champlon <mathieu.champlon@docker.com>

Deprecate Kubernetes context support

a033cdf

Signed-off-by: Mathieu Champlon <mathieu.champlon@docker.com>

thaJeztah added area/context area/kubernetes impact/changelog impact/deprecation status/2-code-review labels Jul 2, 2021

thaJeztah added this to the 21.xx milestone Jul 2, 2021

thaJeztah added the process/cherry-pick label Jul 2, 2021

thaJeztah approved these changes Jul 2, 2021

View reviewed changes

thaJeztah merged commit f782d13 into docker:master Jul 2, 2021

thaJeztah mentioned this pull request Jul 2, 2021

[20.10 backport] Deprecate Kubernetes stack support #3174

Merged

thaJeztah added process/cherry-picked and removed process/cherry-pick labels Jul 2, 2021

thaJeztah mentioned this pull request Jun 21, 2023

cli/command/context: remove deprecated k8s / orchestrator option-stubs #4366

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate Kubernetes stack support #3167

Deprecate Kubernetes stack support #3167

mat007 commented Jun 29, 2021 •

edited

Loading

codecov-commenter commented Jun 29, 2021 •

edited

Loading

mat007 commented Jun 29, 2021

silvin-lubecki left a comment •

edited

Loading

mat007 commented Jul 1, 2021

thaJeztah Jul 1, 2021

mat007 Jul 1, 2021 •

edited

Loading

thaJeztah Jul 2, 2021

thaJeztah commented Jul 2, 2021

thaJeztah left a comment

Deprecate Kubernetes stack support #3167

Deprecate Kubernetes stack support #3167

Conversation

mat007 commented Jun 29, 2021 • edited Loading

codecov-commenter commented Jun 29, 2021 • edited Loading

Codecov Report

mat007 commented Jun 29, 2021

silvin-lubecki left a comment • edited Loading

Choose a reason for hiding this comment

mat007 commented Jul 1, 2021

thaJeztah Jul 1, 2021

Choose a reason for hiding this comment

mat007 Jul 1, 2021 • edited Loading

Choose a reason for hiding this comment

thaJeztah Jul 2, 2021

Choose a reason for hiding this comment

thaJeztah commented Jul 2, 2021

thaJeztah left a comment

Choose a reason for hiding this comment

mat007 commented Jun 29, 2021 •

edited

Loading

codecov-commenter commented Jun 29, 2021 •

edited

Loading

silvin-lubecki left a comment •

edited

Loading

mat007 Jul 1, 2021 •

edited

Loading