tags existing deployment nodes as "found" #18723
Conversation
openshift-ci-robot
added
the
size/XS
| @@ -259,7 +259,7 @@ func EnsureDeploymentNode(g osgraph.MutableUniqueGraph, deployment *extensions.D | |||
| node := osgraph.EnsureUnique(g, | |||
| nodeName, | |||
| func(node osgraph.Node) graph.Node { | |||
| return &DeploymentNode{Node: node, Deployment: deployment} | |||
| return &DeploymentNode{Node: node, Deployment: deployment, IsFound: true} | |||
There was a problem hiding this comment.
why are we setting this to true? I thought that only got set when we verified its existence in the API
There was a problem hiding this comment.
We set it to false in FindOrCreateSynthetic...Node, when a synthetic node is created if a resource is not able to be retrieved / found.
We appear to set IsFound to true here in Ensure...Node funcs
There was a problem hiding this comment.
not in &StatefulSetNode{node, statefulSet, false}...
There was a problem hiding this comment.
not in &StatefulSetNode{node, statefulSet, false}...
That appears to be a mistake I made in this PR:
https://github.com/openshift/origin/pull/18579/files#diff-5b718bf0b0ba776d85871d51d87dd74a
Ensure...Node functions are called from projectstatus.go for every node that is successfully loaded from the server.
I have updated this PR to correct this.
Additionally, it appears that a ScalingEdgeKind was not being added between hpa nodes and replicaSet nodes. I have updated this PR to address this as well.
|
/test extended_networking_minimal |
juanvallejo
force-pushed
the
jvallejo/mark-found-deployment-nodes
branch
from
fd95fb8 to
8bd8e3b
Compare
|
/test cmd |
|
can you add a test that exercises this function that would have failed on the previous bugs? (see existing tests in projectstatus_test.go as examples) |
openshift-ci-robot
added
size/L
|
@liggitt thanks, added a test. Also, updated PR to address the last comment in https://bugzilla.redhat.com/show_bug.cgi?id=1540560#c7 which calls for displaying statefulsets and daemonsets as part of |
|
/retest |
|
@liggitt friendly ping |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: juanvallejo, liggitt The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
/test extended_networking_minimal |
|
Automatic merge from submit-queue. |
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1544183#c3
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1540560#c7
Followup to: #18579
cc @soltysh