Merge pull request #9867 from smarterclayton/cleanup_debug

Merged by openshift-bot

docs/generated/oc_by_example_content.adoc

@@ -2130,7 +2130,7 @@ Generate a new token for a service account.
   # Generate a new token for service account 'default'
   oc serviceaccounts new-token 'default'
 
-  # Generate a new token for service account 'default' and apply 
+  # Generate a new token for service account 'default' and apply
   # labels 'foo' and 'bar' to the new token for identification
   # oc serviceaccounts new-token 'default' --labels foo=foo-value,bar=bar-value
 

docs/man/man1/oc-serviceaccounts-new-token.1

@@ -107,7 +107,7 @@ be applied to any created token so that tokens created with this command can be
   # Generate a new token for service account 'default'
   oc serviceaccounts new\-token 'default'
 
-  # Generate a new token for service account 'default' and apply 
+  # Generate a new token for service account 'default' and apply
   # labels 'foo' and 'bar' to the new token for identification
   # oc serviceaccounts new\-token 'default' \-\-labels foo=foo\-value,bar=bar\-value
 

docs/man/man1/openshift-cli-serviceaccounts-new-token.1

@@ -107,7 +107,7 @@ be applied to any created token so that tokens created with this command can be
   # Generate a new token for service account 'default'
   openshift cli serviceaccounts new\-token 'default'
 
-  # Generate a new token for service account 'default' and apply 
+  # Generate a new token for service account 'default' and apply
   # labels 'foo' and 'bar' to the new token for identification
   # openshift cli serviceaccounts new\-token 'default' \-\-labels foo=foo\-value,bar=bar\-value
 

pkg/cmd/cli/cmd/debug.go

@@ -14,20 +14,20 @@ import (
 	kapierrors "k8s.io/kubernetes/pkg/api/errors"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/client/restclient"
+	kclient "k8s.io/kubernetes/pkg/client/unversioned"
 	"k8s.io/kubernetes/pkg/fields"
 	kcmd "k8s.io/kubernetes/pkg/kubectl/cmd"
 	kcmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	"k8s.io/kubernetes/pkg/kubectl/resource"
+	"k8s.io/kubernetes/pkg/runtime"
 	"k8s.io/kubernetes/pkg/util/interrupt"
 	"k8s.io/kubernetes/pkg/util/term"
-	"k8s.io/kubernetes/pkg/util/wait"
 	"k8s.io/kubernetes/pkg/watch"
 
 	cmdutil "github.com/openshift/origin/pkg/cmd/util"
 	"github.com/openshift/origin/pkg/cmd/util/clientcmd"
-	"github.com/openshift/origin/pkg/generate/app"
+	generateapp "github.com/openshift/origin/pkg/generate/app"
 	imageapi "github.com/openshift/origin/pkg/image/api"
-	"k8s.io/kubernetes/pkg/runtime"
 )
 
 type DebugOptions struct {
@@ -102,7 +102,7 @@ the shell.`
 // NewCmdDebug creates a command for debugging pods.
 func NewCmdDebug(fullName string, f *clientcmd.Factory, in io.Reader, out, errout io.Writer) *cobra.Command {
 	options := &DebugOptions{
-		Timeout: 30 * time.Second,
+		Timeout: 15 * time.Minute,
 		Attach: kcmd.AttachOptions{
 			In:    in,
 			Out:   out,
@@ -238,7 +238,7 @@ func (o *DebugOptions) Complete(cmd *cobra.Command, f *clientcmd.Factory, args [
 		ObjectMeta: template.ObjectMeta,
 		Spec:       template.Spec,
 	}
-	pod.Name, pod.Namespace = infos[0].Name, infos[0].Namespace
+	pod.Name, pod.Namespace = fmt.Sprintf("%s-debug", generateapp.MakeSimpleName(infos[0].Name)), infos[0].Namespace
 	o.Attach.Pod = pod
 
 	o.AsNonRoot = !o.AsRoot && cmd.Flag("as-root").Changed
@@ -288,106 +288,6 @@ func (o DebugOptions) Validate() error {
 	return nil
 }
 
-// WatchConditionFunc returns true if the condition has been reached, false if it has not been reached yet,
-// or an error if the condition cannot be checked and should terminate.
-type WatchConditionFunc func(event watch.Event) (bool, error)
-
-// Until reads items from the watch until each provided condition succeeds, and then returns the last watch
-// encountered. The first condition that returns an error terminates the watch (and the event is also returned).
-// If no event has been received, the returned event will be nil.
-// TODO: move to pkg/watch upstream
-func Until(timeout time.Duration, watcher watch.Interface, conditions ...WatchConditionFunc) (*watch.Event, error) {
-	ch := watcher.ResultChan()
-	defer watcher.Stop()
-	var after <-chan time.Time
-	if timeout > 0 {
-		after = time.After(timeout)
-	} else {
-		ch := make(chan time.Time)
-		close(ch)
-		after = ch
-	}
-	var lastEvent *watch.Event
-	for _, condition := range conditions {
-		for {
-			select {
-			case event, ok := <-ch:
-				if !ok {
-					return lastEvent, wait.ErrWaitTimeout
-				}
-				lastEvent = &event
-				// TODO: check for watch expired error and retry watch from latest point?
-				done, err := condition(event)
-				if err != nil {
-					return lastEvent, err
-				}
-				if done {
-					return lastEvent, nil
-				}
-			case <-after:
-				return lastEvent, wait.ErrWaitTimeout
-			}
-		}
-	}
-	return lastEvent, wait.ErrWaitTimeout
-}
-
-// ErrPodCompleted is returned by PodRunning or PodContainerRunning to indicate that
-// the pod has already reached completed state.
-var ErrPodCompleted = fmt.Errorf("pod ran to completion")
-
-// TODO: move to pkg/client/conditions.go upstream
-//
-// Example of a running condition, will be used elsewhere
-//
-// PodRunning returns true if the pod is running, false if the pod has not yet reached running state,
-// returns ErrPodCompleted if the pod has run to completion, or an error in any other case.
-// func PodRunning(event watch.Event) (bool, error) {
-// 	switch event.Type {
-// 	case watch.Deleted:
-// 		return false, kapierrors.NewNotFound(unversioned.GroupResource{Resource: "pods"}, "")
-// 	}
-// 	switch t := event.Object.(type) {
-// 	case *kapi.Pod:
-// 		switch t.Status.Phase {
-// 		case kapi.PodRunning:
-// 			return true, nil
-// 		case kapi.PodFailed, kapi.PodSucceeded:
-// 			return false, ErrPodCompleted
-// 		}
-// 	}
-// 	return false, nil
-// }
-
-// 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) WatchConditionFunc {
-	return func(event watch.Event) (bool, error) {
-		switch event.Type {
-		case watch.Deleted:
-			return false, kapierrors.NewNotFound(unversioned.GroupResource{Resource: "pods"}, "")
-		}
-		switch t := event.Object.(type) {
-		case *kapi.Pod:
-			switch t.Status.Phase {
-			case kapi.PodRunning, kapi.PodPending:
-			case kapi.PodFailed, kapi.PodSucceeded:
-				return false, ErrPodCompleted
-			default:
-				return false, nil
-			}
-			for _, s := range t.Status.ContainerStatuses {
-				if s.Name != containerName {
-					continue
-				}
-				return s.State.Running != nil, nil
-			}
-			return false, nil
-		}
-		return false, nil
-	}
-}
-
 // SingleObject returns a ListOptions for watching a single object.
 // TODO: move to pkg/api/helpers.go upstream.
 func SingleObject(meta kapi.ObjectMeta) kapi.ListOptions {
@@ -439,7 +339,7 @@ func (o *DebugOptions) Debug() error {
 			return err
 		}
 		fmt.Fprintf(o.Attach.Err, "Waiting for pod to start ...\n")
-		switch containerRunningEvent, err := Until(o.Timeout, w, PodContainerRunning(o.Attach.ContainerName)); {
+		switch containerRunningEvent, err := watch.Until(o.Timeout, w, kclient.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)
@@ -448,7 +348,7 @@ func (o *DebugOptions) Debug() error {
 			}
 			return fmt.Errorf(msg)
 		// switch to logging output
-		case err == ErrPodCompleted, !o.Attach.Stdin:
+		case err == kclient.ErrPodCompleted, !o.Attach.Stdin:
 			_, err := kcmd.LogsOptions{
 				Object: pod,
 				Options: &kapi.PodLogOptions{
@@ -566,9 +466,6 @@ func (o *DebugOptions) transformPodForDebug(annotations map[string]string) (*kap
 	pod.ResourceVersion = ""
 	pod.Spec.RestartPolicy = kapi.RestartPolicyNever
 
-	// shorten segments to handle long names and names with bad characters
-	pod.Name, _ = app.NewUniqueNameGenerator(fmt.Sprintf("%s-debug", pod.Name)).Generate(nil)
-
 	pod.Status = kapi.PodStatus{}
 	pod.UID = ""
 	pod.CreationTimestamp = unversioned.Time{}

pkg/cmd/cli/sa/newtoken.go

@@ -17,7 +17,6 @@ import (
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	"k8s.io/kubernetes/pkg/watch"
 
-	"github.com/openshift/origin/pkg/cmd/cli/cmd"
 	"github.com/openshift/origin/pkg/cmd/util"
 	"github.com/openshift/origin/pkg/cmd/util/clientcmd"
 	"github.com/openshift/origin/pkg/serviceaccounts"
@@ -44,7 +43,7 @@ be applied to any created token so that tokens created with this command can be
 	newServiceAccountTokenExamples = `  # Generate a new token for service account 'default'
   %[1]s 'default'
 
-  # Generate a new token for service account 'default' and apply 
+  # Generate a new token for service account 'default' and apply
   # labels 'foo' and 'bar' to the new token for identification
   # %[1]s 'default' --labels foo=foo-value,bar=bar-value
 `
@@ -201,7 +200,7 @@ func waitForToken(token *api.Secret, serviceAccount *api.ServiceAccount, timeout
 		return nil, fmt.Errorf("could not begin watch for token: %v", err)
 	}
 
-	event, err := cmd.Until(timeout, watcher, func(event watch.Event) (bool, error) {
+	event, err := watch.Until(timeout, watcher, func(event watch.Event) (bool, error) {
 		if event.Type == watch.Error {
 			return false, fmt.Errorf("encountered error while watching for token: %v", event.Object)
 		}

pkg/cmd/util/clientcmd/factory.go

@@ -612,7 +612,17 @@ func (w *Factory) ApproximatePodTemplateForObject(object runtime.Object) (*api.P
 				},
 			},
 		}, nil
-
+	case *imageapi.ImageStreamImage:
+		// create a minimal pod spec that uses the image referenced by the istag without any introspection
+		// it possible that we could someday do a better job introspecting it
+		return &api.PodTemplateSpec{
+			Spec: api.PodSpec{
+				RestartPolicy: api.RestartPolicyNever,
+				Containers: []api.Container{
+					{Name: "container-00", Image: t.Image.DockerImageReference},
+				},
+			},
+		}, nil
 	case *deployapi.DeploymentConfig:
 		fallback := t.Spec.Template
 

pkg/generate/app/pipeline.go

@@ -254,20 +254,27 @@ func (g PipelineGroup) String() string {
 	return strings.Join(s, "+")
 }
 
+// MakeSimpleName strips any non-alphanumeric characters out of a string and returns
+// either an empty string or a string which is valid for most Kubernetes resources.
+func MakeSimpleName(name string) string {
+	name = strings.ToLower(name)
+	name = invalidServiceChars.ReplaceAllString(name, "")
+	name = strings.TrimFunc(name, func(r rune) bool { return r == '-' })
+	if len(name) > kuval.DNS952LabelMaxLength {
+		name = name[:kuval.DNS952LabelMaxLength]
+	}
+	return name
+}
+
 var invalidServiceChars = regexp.MustCompile("[^-a-z0-9]")
 
 func makeValidServiceName(name string) (string, string) {
 	if len(validation.ValidateServiceName(name, false)) == 0 {
 		return name, ""
 	}
-	name = strings.ToLower(name)
-	name = invalidServiceChars.ReplaceAllString(name, "")
-	name = strings.TrimFunc(name, func(r rune) bool { return r == '-' })
-	switch {
-	case len(name) == 0:
+	name = MakeSimpleName(name)
+	if len(name) == 0 {
 		return "", "service-"
-	case len(name) > kuval.DNS952LabelMaxLength:
-		name = name[:kuval.DNS952LabelMaxLength]
 	}
 	return name, ""
 }

test/cmd/debug.sh

@@ -42,6 +42,8 @@ os::cmd::expect_success_and_not_text "oc debug -f examples/hello-openshift/hello
 os::cmd::expect_success 'oc create -f examples/image-streams/image-streams-centos7.json'
 os::cmd::try_until_success 'oc get imagestreamtags wildfly:latest'
 os::cmd::expect_success_and_text "oc debug istag/wildfly:latest -o yaml" 'image: openshift/wildfly-100-centos7'
+sha="$( oc get istag/wildfly:latest --template '{{ .image.metadata.name }}' )"
+os::cmd::expect_success_and_text "oc debug isimage/wildfly@${sha} -o yaml" 'image: openshift/wildfly-100-centos7'
 
 echo "debug: ok"
 os::test::junit::declare_suite_end

test/util/namespace.go

@@ -8,7 +8,6 @@ import (
 	kclient "k8s.io/kubernetes/pkg/client/unversioned"
 	"k8s.io/kubernetes/pkg/watch"
 
-	"github.com/openshift/origin/pkg/cmd/cli/cmd"
 	"github.com/openshift/origin/pkg/cmd/util"
 )
 
@@ -46,7 +45,7 @@ func DeleteAndWaitForNamespaceTermination(c *kclient.Client, name string) error
 	if err := c.Namespaces().Delete(name); err != nil {
 		return err
 	}
-	_, err = cmd.Until(30*time.Second, w, func(event watch.Event) (bool, error) {
+	_, err = watch.Until(30*time.Second, w, func(event watch.Event) (bool, error) {
 		if event.Type != watch.Deleted {
 			return false, nil
 		}