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

Debug should not have 15s timeout #9867

Merged
merged 1 commit into from
Jul 20, 2016
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
2 changes: 1 addition & 1 deletion docs/generated/oc_by_example_content.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion docs/man/man1/oc-serviceaccounts-new-token.1
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion docs/man/man1/openshift-cli-serviceaccounts-new-token.1
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
117 changes: 7 additions & 110 deletions pkg/cmd/cli/cmd/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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{
Expand Down Expand Up @@ -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{}
Expand Down
5 changes: 2 additions & 3 deletions pkg/cmd/cli/sa/newtoken.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
`
Expand Down Expand Up @@ -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)
}
Expand Down
12 changes: 11 additions & 1 deletion pkg/cmd/util/clientcmd/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,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

Expand Down
21 changes: 14 additions & 7 deletions pkg/generate/app/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, ""
}
Expand Down
2 changes: 2 additions & 0 deletions test/cmd/debug.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 1 addition & 2 deletions test/util/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
}
Expand Down