Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix debug panic #20550

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@ import (
"time"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/watch"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/kubectl"

appsv1 "github.com/openshift/api/apps/v1"
Expand Down Expand Up @@ -100,7 +104,7 @@ This may be a transient error. Check the master API logs for anomalies near this
}
defer stopWatcher(watcher)
for event := range watcher.ResultChan() {
running, err := kubectl.PodContainerRunning(d.appName)(event)
running, err := podContainerRunning(d.appName)(event)
if err != nil {
d.out.Error("DCluAC009", err, fmt.Sprintf(`
%s: Error while watching for app pod to deploy:
Expand All @@ -124,3 +128,44 @@ There are many reasons why this can occur; for example:
`, now(), d.deployTimeout))
return false
}

// podContainerRunning returns false until the named container has ContainerStatus running (at least once),
// and will return an error if the pod is deleted, runs to completion, or the container pod is not available.
func podContainerRunning(containerName string) watch.ConditionFunc {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@deads2k this is a copy of the helper under oc/lib/conditions. This diagnostics file deals with internalversions, and we'll be removing diagnostics soon anyway, so I figured we could leave this copy here until then

return func(event watch.Event) (bool, error) {
switch event.Type {
case watch.Deleted:
return false, errors.NewNotFound(schema.GroupResource{Resource: "pods"}, "")
}
switch t := event.Object.(type) {
case *api.Pod:
switch t.Status.Phase {
case api.PodRunning, api.PodPending:
case api.PodFailed, api.PodSucceeded:
return false, kubectl.ErrPodCompleted
default:
return false, nil
}
for _, s := range t.Status.ContainerStatuses {
if s.Name != containerName {
continue
}
if s.State.Terminated != nil {
return false, kubectl.ErrContainerTerminated
}
return s.State.Running != nil, nil
}
for _, s := range t.Status.InitContainerStatuses {
if s.Name != containerName {
continue
}
if s.State.Terminated != nil {
return false, kubectl.ErrContainerTerminated
}
return s.State.Running != nil, nil
}
return false, nil
}
return false, nil
}
}
17 changes: 11 additions & 6 deletions pkg/oc/cli/debug/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import (
appsutil "github.com/openshift/origin/pkg/apps/util"
imageapi "github.com/openshift/origin/pkg/image/apis/image"
imageutil "github.com/openshift/origin/pkg/image/util"
conditions "github.com/openshift/origin/pkg/oc/lib/conditions"
generateapp "github.com/openshift/origin/pkg/oc/lib/newapp/app"
utilenv "github.com/openshift/origin/pkg/oc/util/env"
)
Expand Down Expand Up @@ -103,8 +104,9 @@ type DebugOptions struct {
AppsClient appsv1client.AppsV1Interface
ImageClient imagev1client.ImageV1Interface

Printer printers.ResourcePrinter
LogsForObject polymorphichelpers.LogsForObjectFunc
Printer printers.ResourcePrinter
LogsForObject polymorphichelpers.LogsForObjectFunc
RESTClientGetter genericclioptions.RESTClientGetter

NoStdin bool
ForceTTY bool
Expand Down Expand Up @@ -211,6 +213,7 @@ func (o *DebugOptions) Complete(cmd *cobra.Command, f kcmdutil.Factory, args []s
return kcmdutil.UsageErrorf(cmd, "all resources must be specified before environment changes: %s", strings.Join(args, " "))
}
o.Resources = resources
o.RESTClientGetter = f

switch {
case o.ForceTTY && o.NoStdin:
Expand Down Expand Up @@ -432,7 +435,7 @@ func (o *DebugOptions) RunDebug() error {
}
fmt.Fprintf(o.ErrOut, "Waiting for pod to start ...\n")

switch containerRunningEvent, err := watch.Until(o.Timeout, w, kubectl.PodContainerRunning(o.Attach.ContainerName)); {
switch containerRunningEvent, err := watch.Until(o.Timeout, w, conditions.PodContainerRunning(o.Attach.ContainerName)); {
// api didn't error right away but the pod wasn't even created
case kapierrors.IsNotFound(err):
msg := fmt.Sprintf("unable to create the debug pod %q", pod.Name)
Expand All @@ -444,12 +447,14 @@ func (o *DebugOptions) RunDebug() error {
case err == kubectl.ErrPodCompleted, err == kubectl.ErrContainerTerminated, !o.Attach.Stdin:
return kcmd.LogsOptions{
Object: pod,
Options: &kapi.PodLogOptions{
Options: &corev1.PodLogOptions{
Container: o.Attach.ContainerName,
Follow: true,
},
IOStreams: o.IOStreams,
LogsForObject: o.LogsForObject,
RESTClientGetter: o.RESTClientGetter,
ConsumeRequestFn: kcmd.DefaultConsumeRequestFn,
IOStreams: o.IOStreams,
LogsForObject: o.LogsForObject,
}.RunLogs()
case err != nil:
return err
Expand Down
19 changes: 13 additions & 6 deletions pkg/oc/cli/newapp/newapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,11 @@ type ObjectGeneratorOptions struct {
}

type AppOptions struct {
genericclioptions.IOStreams
*ObjectGeneratorOptions

RESTClientGetter genericclioptions.RESTClientGetter

genericclioptions.IOStreams
}

type versionedPrintObj struct {
Expand Down Expand Up @@ -367,6 +370,8 @@ func NewCmdNewApplication(name, baseName string, f kcmdutil.Factory, streams gen

// Complete sets any default behavior for the command
func (o *AppOptions) Complete(baseName, commandName string, f kcmdutil.Factory, c *cobra.Command, args []string) error {
o.RESTClientGetter = f

cmdutil.WarnAboutCommaSeparation(o.ErrOut, o.ObjectGeneratorOptions.Config.TemplateParameters, "--param")
err := o.ObjectGeneratorOptions.Complete(baseName, commandName, f, c, args)
if err != nil {
Expand Down Expand Up @@ -510,7 +515,7 @@ func (o *AppOptions) RunNewApp() error {

switch {
case len(installing) == 1:
return followInstallation(config, installing[0], o.LogsForObject)
return followInstallation(config, o.RESTClientGetter, installing[0], o.LogsForObject)
case len(installing) > 1:
for i := range installing {
fmt.Fprintf(out, "%sTrack installation of %s with '%s logs %s'.\n", indent, installing[i].Name, o.BaseName, installing[i].Name)
Expand Down Expand Up @@ -554,7 +559,7 @@ func getServices(items []runtime.Object) []*corev1.Service {
return svc
}

func followInstallation(config *newcmd.AppConfig, pod *corev1.Pod, logsForObjectFn polymorphichelpers.LogsForObjectFunc) error {
func followInstallation(config *newcmd.AppConfig, clientGetter genericclioptions.RESTClientGetter, pod *corev1.Pod, logsForObjectFn polymorphichelpers.LogsForObjectFunc) error {
fmt.Fprintf(config.Out, "--> Installing ...\n")

// we cannot retrieve logs until the pod is out of pending
Expand All @@ -567,12 +572,14 @@ func followInstallation(config *newcmd.AppConfig, pod *corev1.Pod, logsForObject
opts := &kcmd.LogsOptions{
Namespace: pod.Namespace,
ResourceArg: pod.Name,
Options: &kapi.PodLogOptions{
Options: &corev1.PodLogOptions{
Follow: true,
Container: pod.Spec.Containers[0].Name,
},
LogsForObject: logsForObjectFn,
IOStreams: genericclioptions.IOStreams{Out: config.Out},
RESTClientGetter: clientGetter,
ConsumeRequestFn: kcmd.DefaultConsumeRequestFn,
LogsForObject: logsForObjectFn,
IOStreams: genericclioptions.IOStreams{Out: config.Out},
}
logErr := opts.RunLogs()

Expand Down
50 changes: 50 additions & 0 deletions pkg/oc/lib/conditions/conditions.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package conditions

import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/watch"
"k8s.io/kubernetes/pkg/kubectl"
)

// PodContainerRunning returns false until the named container has ContainerStatus running (at least once),
// and will return an error if the pod is deleted, runs to completion, or the container pod is not available.
func PodContainerRunning(containerName string) watch.ConditionFunc {
return func(event watch.Event) (bool, error) {
switch event.Type {
case watch.Deleted:
return false, errors.NewNotFound(schema.GroupResource{Resource: "pods"}, "")
}
switch t := event.Object.(type) {
case *corev1.Pod:
switch t.Status.Phase {
case corev1.PodRunning, corev1.PodPending:
case corev1.PodFailed, corev1.PodSucceeded:
return false, kubectl.ErrPodCompleted
default:
return false, nil
}
for _, s := range t.Status.ContainerStatuses {
if s.Name != containerName {
continue
}
if s.State.Terminated != nil {
return false, kubectl.ErrContainerTerminated
}
return s.State.Running != nil, nil
}
for _, s := range t.Status.InitContainerStatuses {
if s.Name != containerName {
continue
}
if s.State.Terminated != nil {
return false, kubectl.ErrContainerTerminated
}
return s.State.Running != nil, nil
}
return false, nil
}
return false, nil
}
}
4 changes: 2 additions & 2 deletions vendor/k8s.io/kubernetes/pkg/kubectl/cmd/logs.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vendor/k8s.io/kubernetes/pkg/kubectl/cmd/run.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.