oc: enable following deployment logs in deploy #9482

0xmichalis · 2016-06-22T11:53:01Z

@smarterclayton @fabianofranz @mfojtik @ironcladlou

0xmichalis · 2016-06-22T11:56:25Z

[test]

mfojtik · 2016-06-22T12:06:23Z

pkg/cmd/cli/cmd/deploy.go

+	opts := deployapi.DeploymentLogOptions{
+		Follow: true,
+	}
+	readCloser, err := o.osClient.DeploymentLogs(config.Namespace).Get(config.Name, opts).Stream()


I assume this will wait till the logs are available (iow. the deployer pod is started).

Currently it just wait until the replication controller is created. I will open a separate issue to make it wait for the deployer pod to come up.

0xmichalis · 2016-06-22T16:30:08Z

flaked on #9490, [test]

smarterclayton · 2016-06-22T23:12:48Z

pkg/deploy/registry/deploylog/rest.go

@@ -126,10 +133,9 @@ func (r *REST) Get(ctx kapi.Context, name string, opts runtime.Object) (runtime.

 	// Get desired deployment
 	targetName := deployutil.DeploymentNameForConfigVersion(config.Name, desiredVersion)
-	target, err := r.rn.ReplicationControllers(namespace).Get(targetName)
+	target, err := r.waitForExistingDeployment(namespace, targetName)


Timeout should come from opts, not a constant

There is a separate issue about wiring a timeout.

Can you link it here?

0xmichalis · 2016-06-29T12:19:15Z

@smarterclayton @mfojtik anything else here?

mfojtik · 2016-06-29T13:36:07Z

pkg/deploy/registry/deploylog/rest.go

-const defaultTimeout time.Duration = 20 * time.Second
+const (
+	// defaultTimeout is the default time to wait for the logs of a deployment.
+	defaultTimeout time.Duration = 20 * time.Second


what error code we return when this timeout is reached? can client retry getting the logs based on the return code?

what error code we return when this timeout is reached?

Depends on where do we reach it. If we timeout while waiting for the replication controller to be created, we return a notFound error because we were polling on that error. If we reach this while waiting for the replication controller to become running then we return a timeout error.

can client retry getting the logs based on the return code?

It's possible. Do we want to do something for oc?

We should return NewServerTimeout

On Jun 29, 2016, at 6:36 AM, Michal Fojtik [email protected] wrote:

In pkg/deploy/registry/deploylog/rest.go
#9482 (comment):

@@ -25,8 +26,12 @@ import (
deployutil "github.com/openshift/origin/pkg/deploy/util"
)

-// defaultTimeout is the default time to wait for the logs of a deployment
-const defaultTimeout time.Duration = 20 * time.Second
+const (

// defaultTimeout is the default time to wait for the logs of a deployment.

defaultTimeout time.Duration = 20 * time.Second

what error code we return when this timeout is reached? can client retry
getting the logs based on the return code?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/openshift/origin/pull/9482/files/53571d0332a00a9aa0df0515d34a867b65273f42#r68946411,
or mute the thread
https://github.com/notifications/unsubscribe/ABG_p6CAHrUsezCmTKGkRaQOC9fxrSlEks5qQnTJgaJpZM4I7sgr
.

Hrm. Maybe. The client will actually retry ServerTimeout in most cases.

On Jun 29, 2016, at 3:33 PM, Michail Kargakis [email protected]
wrote:

In pkg/deploy/registry/deploylog/rest.go
#9482 (comment):

@@ -25,8 +26,12 @@ import (
deployutil "github.com/openshift/origin/pkg/deploy/util"
)

-// defaultTimeout is the default time to wait for the logs of a deployment
-const defaultTimeout time.Duration = 20 * time.Second
+const (

// defaultTimeout is the default time to wait for the logs of a deployment.

defaultTimeout time.Duration = 20 * time.Second

what error code we return when this timeout is reached?

Depends on where do we reach it. If we timeout while waiting for the
replication controller to be created, we return a notFound error because we
were polling on that error. If we reach this while waiting for the
replication controller to become running then we return a timeout error.

can client retry getting the logs based on the return code?

It's possible. Do we want to do something for oc?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/openshift/origin/pull/9482/files/53571d0332a00a9aa0df0515d34a867b65273f42#r69040670,
or mute the thread
https://github.com/notifications/unsubscribe/ABG_p2fpUwlnuRCLLuZW7te7tmbk7kN-ks5qQvKlgaJpZM4I7sgr
.

Updated to return a ServerTimeout error in case we are waiting for the deployment to transition to Running. PTAL

mfojtik · 2016-06-29T13:36:37Z

@Kargakis LGTM one more question/hint.

0xmichalis · 2016-07-06T07:49:50Z

[test]

0xmichalis · 2016-07-06T09:47:52Z

@smarterclayton anything else?

openshift-bot · 2016-07-19T09:55:31Z

Evaluated for origin test up to 11e813f

openshift-bot · 2016-07-19T11:05:31Z

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6437/)

mfojtik · 2016-07-20T14:42:58Z

LGTM

0xmichalis · 2016-07-20T14:45:03Z

[merge]

0xmichalis · 2016-07-20T19:32:07Z

secrets flake [merge]

0xmichalis · 2016-07-21T06:59:55Z

secrets [merge]

0xmichalis · 2016-07-22T11:07:09Z

• Failure [207.665 seconds]
deploymentconfigs
/data/src/github.com/openshift/origin/test/extended/deployments/deployments.go:598
  generation
  /data/src/github.com/openshift/origin/test/extended/deployments/deployments.go:384
    should deploy based on a status version bump [Conformance] [It]
    /data/src/github.com/openshift/origin/test/extended/deployments/deployments.go:383

    Expected error:
        <*errors.errorString | 0xc8200ce130>: {
            s: "timed out waiting for the condition",
        }
        timed out waiting for the condition
    not to have occurred

    /data/src/github.com/openshift/origin/test/extended/deployments/deployments.go:352
------------------------------

[merge]

openshift-bot · 2016-07-22T17:00:56Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6727/) (Image: devenv-rhel7_4646)

0xmichalis · 2016-07-22T17:04:00Z

secrets, hung conformance, and appliedclusterresourcequota [merge]

openshift-bot · 2016-07-22T17:05:57Z

Evaluated for origin merge up to 11e813f

mfojtik reviewed Jun 22, 2016
View reviewed changes

smarterclayton reviewed Jun 22, 2016
View reviewed changes

mfojtik reviewed Jun 29, 2016
View reviewed changes

0xmichalis added 2 commits July 19, 2016 11:50

deploy: update the log registry to poll on notfound deployments

f788c52

oc: enable following deployment logs in deploy

11e813f

openshift-bot merged commit f924e3b into openshift:master Jul 22, 2016

0xmichalis deleted the deploy-follow branch July 22, 2016 19:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

oc: enable following deployment logs in deploy #9482

oc: enable following deployment logs in deploy #9482

0xmichalis commented Jun 22, 2016

0xmichalis commented Jun 22, 2016

mfojtik Jun 22, 2016

0xmichalis Jun 24, 2016

0xmichalis Jun 29, 2016

0xmichalis commented Jun 22, 2016

smarterclayton Jun 22, 2016

0xmichalis Jun 23, 2016

smarterclayton Jun 23, 2016

0xmichalis Jun 23, 2016

0xmichalis commented Jun 29, 2016

mfojtik Jun 29, 2016

0xmichalis Jun 29, 2016

smarterclayton Jun 30, 2016

smarterclayton Jun 30, 2016

0xmichalis Jul 1, 2016

mfojtik commented Jun 29, 2016

0xmichalis commented Jul 6, 2016

0xmichalis commented Jul 6, 2016

openshift-bot commented Jul 19, 2016

openshift-bot commented Jul 19, 2016

mfojtik commented Jul 20, 2016

0xmichalis commented Jul 20, 2016

0xmichalis commented Jul 20, 2016

0xmichalis commented Jul 21, 2016

0xmichalis commented Jul 22, 2016

openshift-bot commented Jul 22, 2016 •

edited

Loading

0xmichalis commented Jul 22, 2016

openshift-bot commented Jul 22, 2016

oc: enable following deployment logs in deploy #9482

oc: enable following deployment logs in deploy #9482

Conversation

0xmichalis commented Jun 22, 2016

0xmichalis commented Jun 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xmichalis commented Jun 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xmichalis commented Jun 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mfojtik commented Jun 29, 2016

0xmichalis commented Jul 6, 2016

0xmichalis commented Jul 6, 2016

openshift-bot commented Jul 19, 2016

openshift-bot commented Jul 19, 2016

mfojtik commented Jul 20, 2016

0xmichalis commented Jul 20, 2016

0xmichalis commented Jul 20, 2016

0xmichalis commented Jul 21, 2016

0xmichalis commented Jul 22, 2016

openshift-bot commented Jul 22, 2016 • edited Loading

0xmichalis commented Jul 22, 2016

openshift-bot commented Jul 22, 2016

openshift-bot commented Jul 22, 2016 •

edited

Loading