Bug 1760828: update Active RC logic to not show Failed deployments #3627

sahil143 · 2019-12-02T12:28:28Z

ODC Bug: https://jira.coreos.com/browse/ODC-1412

This PR updates the logic to get the active replication controller.
Steps:

Get all the RCs for a resource.
Sort them based on the revision.
Get the first RC(most recent) from the sorted list to add the alerts if it's failed or canceled
filter out the RCs based on status.replicas or if an RC is in New, Pending, Running or Complete phase to determine active RCs.

using the logic from 3.11 code: https://github.com/openshift/origin-web-console/blob/enterprise-3.11/app/scripts/controllers/overview.js#L785-L831

fix recent Rc to not be failed/canceled add warnings for latest RC remove console rmeove comments

sahil143 · 2019-12-02T12:39:38Z

/cc @christianvogt @spadgett

openshift-ci-robot · 2019-12-02T18:32:09Z

@sahil143: This pull request references Bugzilla bug 1760828, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Bug 1760828:update Active RC logic to not show Failed deployments

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

christianvogt · 2019-12-04T03:01:21Z

@spadgett Could you please have a look at this as you're much more familiar with the scenario.

christianvogt · 2019-12-04T03:36:22Z

Seems to work in showing the correct data:

openshift-ci-robot · 2019-12-04T04:07:40Z

@sahil143: This pull request references Bugzilla bug 1760828, which is valid.

Details

In response to this:

Bug 1760828: update Active RC logic to not show Failed deployments

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2019-12-04T04:10:46Z

@sahil143: This pull request references Bugzilla bug 1760828, which is valid.

Details

In response to this:

Bug 1760828: update Active RC logic to not show Failed deployments

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

karthikjeeyar · 2019-12-04T08:35:35Z

frontend/packages/console-shared/src/utils/resource-utils.ts

+};
+
+const isReplicationControllerVisible = (resource: K8sResourceKind): boolean => {
+  return _.get(resource, ['status', 'replicas'], false) || deploymentIsInProgress(resource);


we could simply do

Suggested change

return _.get(resource, ['status', 'replicas'], false) || deploymentIsInProgress(resource);

return _.get(resource, ['status', 'replicas'], deploymentIsInProgress(resource));

The return type is lying here since it might return a number instead of a boolean.

Suggested change

return _.get(resource, ['status', 'replicas'], false) || deploymentIsInProgress(resource);

return !!_.get(resource, ['status', 'replicas']) || deploymentIsInProgress(resource);

Won't this return true for all previous "complete" deployments?

fixed in e0d1c43

spadgett · 2019-12-12T19:49:46Z

frontend/packages/console-shared/src/utils/resource-utils.ts

 };

+const getDeploymentConfigName = (obj: K8sResourceKind): string => {
+  return getAnnotation(obj, DEPLOYMENT_CONFIG_NAME_ANNOTATION);


You should use metadata.ownerReferences instead of the annotation.

fixed in e0d1c43

spadgett · 2019-12-12T19:50:46Z

frontend/packages/console-shared/src/utils/resource-utils.ts

+      DEPLOYMENT_PHASE.new,
+      DEPLOYMENT_PHASE.pending,
+      DEPLOYMENT_PHASE.running,
+      DEPLOYMENT_PHASE.complete,


Why is complete considered in progress?

Because we are looking for all phases except for failed or canceled and choose the first two from that filtered array i.e. current and previous. Seems like method name is misleading changed it to isDeploymentInProgressOrCompleted.

spadgett · 2019-12-12T19:50:58Z

frontend/packages/console-shared/src/utils/resource-utils.ts

  return notFailedOrCancelled && previous && previous.pods.length > 0;
 };

+const deploymentIsInProgress = (resource: K8sResourceKind): boolean => {


Suggested change

const deploymentIsInProgress = (resource: K8sResourceKind): boolean => {

const isDeploymentInProgress = (resource: K8sResourceKind): boolean => {

fixed in e0d1c43

spadgett · 2019-12-12T19:53:43Z

frontend/packages/console-shared/src/utils/resource-utils.ts

+};
+
+const isReplicationControllerVisible = (resource: K8sResourceKind): boolean => {
+  return _.get(resource, ['status', 'replicas'], false) || deploymentIsInProgress(resource);


The return type is lying here since it might return a number instead of a boolean.

Suggested change

return _.get(resource, ['status', 'replicas'], false) || deploymentIsInProgress(resource);

return !!_.get(resource, ['status', 'replicas']) || deploymentIsInProgress(resource);

spadgett · 2019-12-12T19:54:41Z

frontend/packages/console-shared/src/utils/resource-utils.ts

+};
+
+const isReplicationControllerVisible = (resource: K8sResourceKind): boolean => {
+  return _.get(resource, ['status', 'replicas'], false) || deploymentIsInProgress(resource);


Won't this return true for all previous "complete" deployments?

…ionControllerVisible

karthikjeeyar · 2019-12-17T16:19:00Z

Verified locally, It works fine
/lgtm

christianvogt · 2019-12-19T19:43:11Z

/approve

openshift-ci-robot · 2019-12-19T19:43:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christianvogt, karthikjeeyar, sahil143

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~frontend/OWNERS~~ [christianvogt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2019-12-20T12:41:12Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-20T12:54:05Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-21T10:59:59Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-21T12:18:14Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-21T12:31:13Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-21T13:35:56Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-21T13:48:57Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-21T14:54:02Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-21T15:06:55Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-21T15:58:56Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-21T17:03:57Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-21T17:16:56Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-21T18:21:57Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-21T18:34:57Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-21T19:40:08Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-21T19:52:57Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-21T20:57:59Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-21T21:10:58Z

/retest

Please review the full test history for this PR and help us cut down flakes.

spadgett · 2019-12-21T21:41:13Z

/hold
e2e-gcp-console is currently broken

andrewballantyne · 2020-01-02T19:42:31Z

/kind bug

spadgett · 2020-01-02T23:28:43Z

/hold cancel
/retest
#3847 has fixed the e2e tests

openshift-bot · 2020-01-03T02:43:16Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-01-03T04:00:52Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-01-03T04:13:55Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-01-03T05:19:45Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-01-03T06:36:49Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2020-01-03T09:39:36Z

@sahil143: All pull requests linked via external trackers have merged. Bugzilla bug 1760828 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1760828: update Active RC logic to not show Failed deployments

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

update Active Rc to show only complete RCs

85ec346

fix recent Rc to not be failed/canceled add warnings for latest RC remove console rmeove comments

openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. component/shared Related to console-shared labels Dec 2, 2019

openshift-ci-robot requested review from jeff-phillips-18 and jhadvig December 2, 2019 12:29

openshift-ci-robot requested review from christianvogt and spadgett December 2, 2019 12:39

sahil143 changed the title ~~update Active RC logic to not show Failed deployments~~ Bug Bugzilla 1760828update Active RC logic to not show Failed deployments Dec 2, 2019

sahil143 changed the title ~~Bug Bugzilla 1760828update Active RC logic to not show Failed deployments~~ Bug 1760828:update Active RC logic to not show Failed deployments Dec 2, 2019

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Dec 2, 2019

sahil143 changed the title ~~Bug 1760828:update Active RC logic to not show Failed deployments~~ Bug 1760828: update Active RC logic to not show Failed deployments Dec 2, 2019

karthikjeeyar reviewed Dec 4, 2019

View reviewed changes

spadgett reviewed Dec 12, 2019

View reviewed changes

change method name, add check for mostRecentRc, return for isReplicat…

e0d1c43

…ionControllerVisible

sahil143 requested review from karthikjeeyar and spadgett December 17, 2019 11:25

openshift-ci-robot assigned karthikjeeyar Dec 17, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 17, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 19, 2019

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 21, 2019

openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jan 2, 2020

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 2, 2020

openshift-merge-robot merged commit 88da676 into openshift:master Jan 3, 2020

spadgett added this to the v4.4 milestone Jan 3, 2020

	return _.get(resource, ['status', 'replicas'], false) \|\| deploymentIsInProgress(resource);
	return _.get(resource, ['status', 'replicas'], deploymentIsInProgress(resource));

	return _.get(resource, ['status', 'replicas'], false) \|\| deploymentIsInProgress(resource);
	return !!_.get(resource, ['status', 'replicas']) \|\| deploymentIsInProgress(resource);

	const deploymentIsInProgress = (resource: K8sResourceKind): boolean => {
	const isDeploymentInProgress = (resource: K8sResourceKind): boolean => {

Bug 1760828: update Active RC logic to not show Failed deployments #3627

Bug 1760828: update Active RC logic to not show Failed deployments #3627

Uh oh!

Conversation

sahil143 commented Dec 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sahil143 commented Dec 2, 2019

Uh oh!

openshift-ci-robot commented Dec 2, 2019

Uh oh!

christianvogt commented Dec 4, 2019

Uh oh!

christianvogt commented Dec 4, 2019

Uh oh!

openshift-ci-robot commented Dec 4, 2019

Uh oh!

openshift-ci-robot commented Dec 4, 2019

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!

Choose a reason for hiding this comment

Uh oh!

karthikjeeyar commented Dec 17, 2019

Uh oh!

christianvogt commented Dec 19, 2019

Uh oh!

openshift-ci-robot commented Dec 19, 2019

Uh oh!

openshift-bot commented Dec 20, 2019

Uh oh!

openshift-bot commented Dec 20, 2019

Uh oh!

openshift-bot commented Dec 21, 2019

Uh oh!

openshift-bot commented Dec 21, 2019

Uh oh!

openshift-bot commented Dec 21, 2019

Uh oh!

openshift-bot commented Dec 21, 2019

Uh oh!

openshift-bot commented Dec 21, 2019

Uh oh!

openshift-bot commented Dec 21, 2019

Uh oh!

openshift-bot commented Dec 21, 2019

Uh oh!

openshift-bot commented Dec 21, 2019

Uh oh!

openshift-bot commented Dec 21, 2019

Uh oh!

openshift-bot commented Dec 21, 2019

Uh oh!

openshift-bot commented Dec 21, 2019

Uh oh!

openshift-bot commented Dec 21, 2019

Uh oh!

openshift-bot commented Dec 21, 2019

Uh oh!

openshift-bot commented Dec 21, 2019

sahil143 commented Dec 2, 2019 •

edited

Loading