CLI: add support for deployments in oc status by mfojtik · Pull Request #18439 · openshift/origin

mfojtik · 2018-02-05T13:30:14Z

@smarterclayton this is a long overdue...

current state:

$ oc status
In project My Project (myproject) on server https://127.0.0.1:8443

svc/ruby-deploy - 172.30.174.234:8080
  deployment/ruby-deploy deploys istag/ruby-deploy:latest <-
    bc/ruby-deploy source builds https://github.com/openshift/ruby-ex.git on istag/ruby-22-centos7:latest
      build #1 failed 5 hours ago - bbb6701: Merge pull request #18 from durandom/master (Ben Parees <bparees@users.noreply.github.com>)
    deployment #2 running for 4 hours - 0/1 pods (warning: 53 restarts)
    deployment #1 deployed 5 hours ago

TODO:

Add rollouts similar to deployment configs
Fix unit tests / Add unit tests
Deal with HPA

mfojtik · 2018-02-05T15:52:20Z

pkg/oc/admin/prune/imageprune/prune.go

 		ref := getRef(d)
 		glog.V(4).Infof("Examining %s", getKindName(ref))
-		dNode := appsgraph.EnsureDeploymentNode(p.g, d)
+		dNode := kubegraph.EnsureDeploymentNode(p.g, d)


if you wonder why, I moved 'Deployments' from appsgraph which seems to hold only apps.openshift.io resources to kubegraph as Deployments are upstream.

Would you mind moving DaemonSets as well, it's the function before this one?

Same for ReplicaSets a few lines below.

interesting, I thought I moved replica sets already... seems like they are not used/loaded anywhere

done. moved both to kube nodes

mfojtik · 2018-02-05T15:53:52Z

pkg/oc/cli/describe/projectstatus.go

+func describeDeploymentStatus(rs *kapisext.ReplicaSet, revision int64, first bool, restartCount int32) string {
+	timeAt := strings.ToLower(formatRelativeTime(rs.CreationTimestamp.Time))
+	replicaSetRevision, _ := deployutil.Revision(rs)
+	if replicaSetRevision == revision {


@Kargakis @tnozicka i really hope there is something better to report the state of RS created/managed by deployment... The only thing that RS contain is revision so I know this RS is "old", but it does not say anything about if it failed/etc.

@spadgett since you did the console pages for deployments

mfojtik · 2018-02-05T15:54:59Z

pkg/oc/graph/kubegraph/edges.go

 	// ScalingEdgeKind goes from HorizontalPodAutoscaler to scaled objects indicating that the HPA scales the object
 	ScalingEdgeKind = "Scaling"
+	// TriggersDeploymentEdgeKind points from DeploymentConfigs to ImageStreamTags that trigger the deployment
+	TriggersDeploymentEdgeKind = "TriggersDeployment"


this was duplicated from appsgraph because of import cycles... we should probably move this into some common package

follow up, don't want to pollute this PR

mfojtik · 2018-02-05T15:55:29Z

pkg/oc/graph/kubegraph/edges.go

 }
+
+// AddTriggerEdges creates edges that point to named Docker image repositories for each image used in the deployment.
+func AddTriggerEdges(g osgraph.MutableUniqueGraph, node *kubegraph.DeploymentNode) *kubegraph.DeploymentNode {


we should rework this to make it usable for StatefulSets and DaemonSets?

done, this now works for statefulsets as well

smarterclayton · 2018-02-05T16:10:48Z

Happy dance On Feb 5, 2018, at 5:30 AM, Michal Fojtik <notifications@github.com> wrote: @smarterclayton <https://github.com/smarterclayton> this is a long overdue... current state: $ oc status In project My Project (myproject) on server https://127.0.0.1:8443 svc/ruby-deploy - 172.30.174.234:8080 deployment/ruby-deploy deploys istag/ruby-deploy:latest <- bc/ruby-deploy source builds https://github.com/openshift/ruby-ex.git on istag/ruby-22-centos7:latest build #1 failed 3 hours ago - bbb6701: Merge pull request #18 from durandom/master (Ben Parees <bparees@users.noreply.github.com>) pod/ruby-deploy-7556484b56-bpsqs runs busybox@sha256:4cee1979ba0bf7db9fc5d28fb7b798ca69ae95a47c5fecf46327720df4ff352d TODO: - Add rollouts similar to deployment configs - Fix unit tests / Add unit tests - Deal with HPA

…

mfojtik · 2018-02-05T16:17:51Z

@smarterclayton ready for review :-)

soltysh

A few suggestions, but this lgtm

soltysh · 2018-02-05T16:37:36Z

pkg/oc/cli/cmd/status.go


 	o.describer = &describe.ProjectStatusDescriber{
-		K:             kclientset,
+		KubeClient:    kclientset,


soltysh · 2018-02-05T16:50:56Z

pkg/oc/admin/prune/imageprune/prune.go

 		ref := getRef(d)
 		glog.V(4).Infof("Examining %s", getKindName(ref))
-		dNode := appsgraph.EnsureDeploymentNode(p.g, d)
+		dNode := kubegraph.EnsureDeploymentNode(p.g, d)


Would you mind moving DaemonSets as well, it's the function before this one?

soltysh · 2018-02-05T16:51:20Z

pkg/oc/admin/prune/imageprune/prune.go

 		ref := getRef(d)
 		glog.V(4).Infof("Examining %s", getKindName(ref))
-		dNode := appsgraph.EnsureDeploymentNode(p.g, d)
+		dNode := kubegraph.EnsureDeploymentNode(p.g, d)


Same for ReplicaSets a few lines below.

soltysh

lgtm, but would you mind removing the wip from the 2nd commit title ;) ?

soltysh · 2018-02-06T10:15:58Z

pkg/oc/cli/describe/projectstatus.go

-func describeStatefulSetInServiceGroup(f formatter, node *kubegraph.StatefulSetNode) []string {
+func describeDeploymentInServiceGroup(f formatter, deploy graphview.Deployment, restartFn func(node *kubegraph.ReplicaSetNode) int32) []string {
+	local := namespacedFormatter{currentNamespace: deploy.Deployment.Deployment.Namespace}
+	// TODO: Figure out what this is


mfojtik · 2018-02-07T09:10:10Z

/retest

soltysh

/lgtm
/approve

openshift-ci-robot · 2018-02-07T12:56:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mfojtik, soltysh

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these OWNERS Files:

~~pkg/oc/OWNERS~~ [mfojtik,soltysh]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

openshift-merge-robot · 2018-02-07T20:44:19Z

Automatic merge from submit-queue.

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 5, 2018

openshift-ci-robot requested review from juanvallejo and soltysh February 5, 2018 13:30

openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 5, 2018

mfojtik force-pushed the oc-status branch from 9d6effe to 8106914 Compare February 5, 2018 15:37

openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 5, 2018

mfojtik commented Feb 5, 2018

View reviewed changes

mfojtik changed the title ~~WIP: add support for deployments in oc status~~ CLI: add support for deployments in oc status Feb 5, 2018

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 5, 2018

soltysh approved these changes Feb 5, 2018

View reviewed changes

mfojtik force-pushed the oc-status branch 2 times, most recently from b035d04 to 86a8bc9 Compare February 5, 2018 19:39

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 5, 2018

cli: fix kube client name

e777abd

mfojtik force-pushed the oc-status branch from 86a8bc9 to 55604e4 Compare February 6, 2018 09:51

soltysh approved these changes Feb 6, 2018

View reviewed changes

mfojtik force-pushed the oc-status branch from 55604e4 to 91c8ee7 Compare February 6, 2018 13:46

mfojtik force-pushed the oc-status branch 2 times, most recently from c587158 to 02412d4 Compare February 7, 2018 09:58

add support for deployments in oc status

e0e0368

mfojtik force-pushed the oc-status branch from 02412d4 to e0e0368 Compare February 7, 2018 10:28

soltysh approved these changes Feb 7, 2018

View reviewed changes

openshift-ci-robot assigned soltysh Feb 7, 2018

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 7, 2018

openshift-merge-robot merged commit 0427a13 into openshift:master Feb 7, 2018

mfojtik deleted the oc-status branch September 5, 2018 21:07

Conversation

mfojtik commented Feb 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smarterclayton commented Feb 5, 2018 via email

Uh oh!

mfojtik commented Feb 5, 2018

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mfojtik commented Feb 7, 2018

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Feb 7, 2018

Uh oh!

openshift-merge-robot commented Feb 7, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mfojtik commented Feb 5, 2018 •

edited

Loading