Unverified Commit 4fff3523 authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub

Merge pull request #67316 from m1kola/67314_fix_regression

Automatic merge from submit-queue (batch tested with PRs 66920, 67316, 67363, 67528, 66963). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Fixes regression in kubectl logs: the --all-containers=true option didn't work **What this PR does / why we need it**: Fixes regression introduced in #66398 and adds unit tests for logging with `--all-containers=true`. See #67314 for more details. **Which issue(s) this PR fixes**: Fixes #67314 **Special notes for your reviewer**: I didn't cover cases with `coreinternal.PodList` and `coreinternal.Pod` in tests, because it doesn't look like we need them: I didn't manage to find any callers of the `logsForObjectWithClient` and `logsForObject` functions, so, probably, we can remove them. I'll double check and try to do that separately once this PR is merged. **Release note**: ```release-note NONE ``` /sig cli
parents 49b29541 89dd809e
...@@ -87,7 +87,7 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt ...@@ -87,7 +87,7 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt
for _, c := range t.Spec.InitContainers { for _, c := range t.Spec.InitContainers {
currOpts := opts.DeepCopy() currOpts := opts.DeepCopy()
currOpts.Container = c.Name currOpts.Container = c.Name
currRet, err := logsForObjectWithClient(clientset, t, options, timeout, false) currRet, err := logsForObjectWithClient(clientset, t, currOpts, timeout, false)
if err != nil { if err != nil {
return nil, err return nil, err
} }
...@@ -96,7 +96,7 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt ...@@ -96,7 +96,7 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt
for _, c := range t.Spec.Containers { for _, c := range t.Spec.Containers {
currOpts := opts.DeepCopy() currOpts := opts.DeepCopy()
currOpts.Container = c.Name currOpts.Container = c.Name
currRet, err := logsForObjectWithClient(clientset, t, options, timeout, false) currRet, err := logsForObjectWithClient(clientset, t, currOpts, timeout, false)
if err != nil { if err != nil {
return nil, err return nil, err
} }
...@@ -115,7 +115,7 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt ...@@ -115,7 +115,7 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt
for _, c := range t.Spec.InitContainers { for _, c := range t.Spec.InitContainers {
currOpts := opts.DeepCopy() currOpts := opts.DeepCopy()
currOpts.Container = c.Name currOpts.Container = c.Name
currRet, err := logsForObjectWithClient(clientset, t, options, timeout, false) currRet, err := logsForObjectWithClient(clientset, t, currOpts, timeout, false)
if err != nil { if err != nil {
return nil, err return nil, err
} }
...@@ -124,7 +124,7 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt ...@@ -124,7 +124,7 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt
for _, c := range t.Spec.Containers { for _, c := range t.Spec.Containers {
currOpts := opts.DeepCopy() currOpts := opts.DeepCopy()
currOpts.Container = c.Name currOpts.Container = c.Name
currRet, err := logsForObjectWithClient(clientset, t, options, timeout, false) currRet, err := logsForObjectWithClient(clientset, t, currOpts, timeout, false)
if err != nil { if err != nil {
return nil, err return nil, err
} }
......
...@@ -40,11 +40,12 @@ var ( ...@@ -40,11 +40,12 @@ var (
func TestLogsForObject(t *testing.T) { func TestLogsForObject(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
obj runtime.Object obj runtime.Object
opts *corev1.PodLogOptions opts *corev1.PodLogOptions
pods []runtime.Object allContainers bool
actions []testclient.Action pods []runtime.Object
actions []testclient.Action
}{ }{
{ {
name: "pod logs", name: "pod logs",
...@@ -57,6 +58,84 @@ func TestLogsForObject(t *testing.T) { ...@@ -57,6 +58,84 @@ func TestLogsForObject(t *testing.T) {
}, },
}, },
{ {
name: "pod logs: all containers",
obj: &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "hello", Namespace: "test"},
Spec: corev1.PodSpec{
InitContainers: []corev1.Container{
{Name: "initc1"},
{Name: "initc2"},
},
Containers: []corev1.Container{
{Name: "c1"},
{Name: "c2"},
},
},
},
opts: &corev1.PodLogOptions{},
allContainers: true,
pods: []runtime.Object{testPod()},
actions: []testclient.Action{
getLogsAction("test", &corev1.PodLogOptions{Container: "initc1"}),
getLogsAction("test", &corev1.PodLogOptions{Container: "initc2"}),
getLogsAction("test", &corev1.PodLogOptions{Container: "c1"}),
getLogsAction("test", &corev1.PodLogOptions{Container: "c2"}),
},
},
{
name: "pods list logs",
obj: &corev1.PodList{
Items: []corev1.Pod{
{
ObjectMeta: metav1.ObjectMeta{Name: "hello", Namespace: "test"},
Spec: corev1.PodSpec{
InitContainers: []corev1.Container{
{Name: "initc1"},
{Name: "initc2"},
},
Containers: []corev1.Container{
{Name: "c1"},
{Name: "c2"},
},
},
},
},
},
pods: []runtime.Object{testPod()},
actions: []testclient.Action{
getLogsAction("test", nil),
},
},
{
name: "pods list logs: all containers",
obj: &corev1.PodList{
Items: []corev1.Pod{
{
ObjectMeta: metav1.ObjectMeta{Name: "hello", Namespace: "test"},
Spec: corev1.PodSpec{
InitContainers: []corev1.Container{
{Name: "initc1"},
{Name: "initc2"},
},
Containers: []corev1.Container{
{Name: "c1"},
{Name: "c2"},
},
},
},
},
},
opts: &corev1.PodLogOptions{},
allContainers: true,
pods: []runtime.Object{testPod()},
actions: []testclient.Action{
getLogsAction("test", &corev1.PodLogOptions{Container: "initc1"}),
getLogsAction("test", &corev1.PodLogOptions{Container: "initc2"}),
getLogsAction("test", &corev1.PodLogOptions{Container: "c1"}),
getLogsAction("test", &corev1.PodLogOptions{Container: "c2"}),
},
},
{
name: "replication controller logs", name: "replication controller logs",
obj: &corev1.ReplicationController{ obj: &corev1.ReplicationController{
ObjectMeta: metav1.ObjectMeta{Name: "hello", Namespace: "test"}, ObjectMeta: metav1.ObjectMeta{Name: "hello", Namespace: "test"},
...@@ -130,11 +209,12 @@ func TestLogsForObject(t *testing.T) { ...@@ -130,11 +209,12 @@ func TestLogsForObject(t *testing.T) {
for _, test := range tests { for _, test := range tests {
fakeClientset := fakeexternal.NewSimpleClientset(test.pods...) fakeClientset := fakeexternal.NewSimpleClientset(test.pods...)
_, err := logsForObjectWithClient(fakeClientset.CoreV1(), test.obj, test.opts, 20*time.Second, false) _, err := logsForObjectWithClient(fakeClientset.CoreV1(), test.obj, test.opts, 20*time.Second, test.allContainers)
if err != nil { if err != nil {
t.Errorf("%s: unexpected error: %v", test.name, err) t.Errorf("%s: unexpected error: %v", test.name, err)
continue continue
} }
for i := range test.actions { for i := range test.actions {
if len(fakeClientset.Actions()) < i { if len(fakeClientset.Actions()) < i {
t.Errorf("%s: action %d does not exists in actual actions: %#v", t.Errorf("%s: action %d does not exists in actual actions: %#v",
...@@ -160,7 +240,10 @@ func testPod() runtime.Object { ...@@ -160,7 +240,10 @@ func testPod() runtime.Object {
Spec: corev1.PodSpec{ Spec: corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyAlways, RestartPolicy: corev1.RestartPolicyAlways,
DNSPolicy: corev1.DNSClusterFirst, DNSPolicy: corev1.DNSClusterFirst,
Containers: []corev1.Container{{Name: "c1"}}, Containers: []corev1.Container{
{Name: "c1"},
{Name: "c2"},
},
}, },
} }
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment