don't use the env flag and deprecate the log annotation in the future

keps/sig-cli/2227-kubectl-default-container/README.md

@@ -81,12 +81,12 @@ Quoted from [kubernetes #96986](https://github.com/kubernetes/kubernetes/issues/
 ### Goals
 
 - Provide a way for consumers (CLI, operators) to know which is the default Container of a Pod
+- Deprecate the already in use annotation `kubectl.kubernetes.io/default-logs-container`
 
 ### Non-Goals
 
 - If the cli is not kubectl, we don't determine which is the default container.
 - Automatically define/create the default container annotation. This is an user operation.
-- Deprecate the already in use annotation `kubectl.kubernetes.io/default-logs-container`
 
 ## Proposal
 
@@ -148,44 +148,20 @@ the client side.
 
 Alpha:  default container annotation
 
-- Define a well known annotation `kubectl.kubernetes.io/default-container` in a Pod to provide a way to consumers (CLI, operators) to known which is the default Container.
+- Define a well known annotation `kubectl.kubernetes.io/default-container` in a Pod to provide a way to consumers (CLI, operators) to know which is the default Container.
 - Define a global function GetDefaultContainerName for kubectl that uses this annotation like below
-```
-func GetDefaultContainerName(p *ExecOptions, pod *corev1.Pod) string{
-  containerName := p.ContainerName
-  if len(containerName) != 0 {
-    return containerName
-  }
-
-  if len(pod.Spec.Containers) > 1 {
-    // in case the "kubectl.kubernetes.io/default-container" annotation is present, we preset the opts.Containers to default to selected
-    // container. This gives users ability to preselect the most interesting container in pod.
-    if annotations := pod.Annotations; annotations != nil && len(annotations[corev1.DefaultContainerAnnotationName]) > 0 {
-      containerName := annotations[corev1.DefaultContainerAnnotationName]
-      if exists, _ := podutils.FindContainerByName(pod, containerName); exists != nil {
-        return containerName
-      } else {
-        fmt.Fprintf(p.ErrOut, "Default container name %q in annotation not found in a pod\n", containerName)
-      }
-    }
-    fmt.Fprintf(p.ErrOut, "Defaulting container name to first container %s.\n", pod.Spec.Containers[0].Name)
-
-    if p.EnableSuggestedCmdUsage {
-      fmt.Fprintf(p.ErrOut, "Use '%s describe pod/%s -n %s' to see all of the containers in this pod.\n", p.ParentCommandName, pod.Name, p.Namespace)
-    }
-  }
-
-  containerName = pod.Spec.Containers[0].Name
-  return containerName
-}
-```
+-- 1. if the command is `logs` check `--all-containers` flag at first: if it is specified, ignore container flag or annotations; if not, next step.
+-- 2. check `-c`/`--container` flag: if it is specified, use it; if not, next step.
+-- 3. check containers number in pod: if only one container, use its name; if more than one, next step.
+-- 4. check annotations: 
+-- 4.1 if command is `log` and pod has `kubectl.kubernetes.io/default-logs-container` annotation, then use the annotation value and print a warning message for deprecation(removing this step when GA); 
+-- 4.2 check `kubectl.kubernetes.io/default-container` annotation: if specified, use it; if not, next step
+-- 5. use the first container name as the default. Print a notice message before it.
 - Add test cases to make sure that the command is running with the right container. When `--container` is specified, the annotation will be ignored.
-- Validate the annotation value before using it, as container name should follow RFC 1123. If the annotation value is invalid or not found in the pod, a warning message is needed before exiting.
-- Add an Env 'KUBE_DEFAULT_CONTAINER_ANNOTATION=disabled', we will ignore the specified annotation. This Env is only for disabling this. By default, this feature should be enabled as this feature is opt-in, and it only works once user adds the specified annotation to their pods.
+- Validate the annotation value before using it, as the container name should follow RFC 1123. If the annotation value is invalid or not found in the pod, a warning message is needed before exiting.
+- By default, this feature should be enabled as this feature is opt-in, and it only works once user adds the specified annotation to their pods.
 - Ensure that when `kubectl.kubernetes.io/default-logs-container` is specified, we should use this annotation instead of the general one for `kubectl log` and use general annotation for other commands.
 
-
-
 **Data Command Structure:**
 
 
@@ -212,12 +188,13 @@ Add a unit test for each command, testing the behavior with the annotation, with
 As this is an opt-in feature, no gate is expected.
 - At least 2 release cycles pass to gather feedback and bug reports during
 - Documentations, add it to [well-known annotations docs](https://kubernetes.io/docs/reference/kubernetes-api/labels-annotations-taints/)
+- Add a warning deprecation message when using the annotation `kubectl.kubernetes.io/default-logs-container`
 
 #### Beta -> GA Graduation
 
 - Gather feedback from developers and surveys
 - At least 2 release cycles pass to gather feedback and bug reports during
-- Once this feature is GA, remove env check of KUBE_DEFAULT_CONTAINER_ANNOTATION. This feature should always be enabled.
+- The deprecation message of the annotation `kubectl.kubernetes.io/default-logs-container` will be removed and this annotation will stop working.
 
 ### Upgrade / Downgrade Strategy
 
@@ -253,11 +230,13 @@ None
 
 * **Can the feature be disabled once it has been enabled (i.e. can we rollback
   the enablement)?**
-  - Yes. When an Env 'KUBE_DEFAULT_CONTAINER_ANNOTATION=disabled', we will ignore
-    the specified annotation. This Env is only for disabling this.
+  - Yes. When `--container` flag is specified, we will ignore
+    the specified annotation.
 
 * **What happens if we reenable the feature if it was previously rolled back?**
-  - Nothing. Just unset Env KUBE_DEFAULT_CONTAINER_ANNOTATION to enable it by default.
+  - Nothing. It uses the first container as default in old behivior. It uses the 
+    annotation with this feature implemented, the first container will be used
+    if the annotation is not defined.
 
 * **Are there any tests for feature enablement/disablement?**
   - There are unit tests in `staging/src/k8s.io/kubectl/pkg/cmd/exec/` and 
@@ -271,7 +250,7 @@ None
 * **What specific metrics should inform a rollback?
   - None
 * **Were upgrade and rollback tested?
-  - 'We're going to test setting the env var KUBE_DEFAULT_CONTAINER_ANNOTATION and then unsetting it.
+  - None, or with adding and deleting the annotation from a pod to test it.
 * **Is the rollout accompanied by any deprecations and/or removals of features, APIs, fields of API types, flags, etc.? Even if applying deprecation policies, they may still surprise some users. No.
   - None