-
Notifications
You must be signed in to change notification settings - Fork 231
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
Add StatefulSets to monitoring #1163
Add StatefulSets to monitoring #1163
Conversation
so the interesting problem here is that when metrics isnt enabled there
would be nothing to show when the row is expanded :-\
…On Fri, Jan 20, 2017 at 1:40 PM, Ben Petersen ***@***.***> wrote:
@benjaminapetersen <https://github.com/benjaminapetersen> requested your
review on: openshift/origin-web-console#1163
<#1163> [WIP] Add
StatefulSets to monitoring.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1163 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABZk7WhjMveGVJkltnGM5bBHAeVCdyydks5rUP-IgaJpZM4LpnVF>
.
|
Here's what we show for replica sets: |
TODO yet:
|
9bf8759
to
875ef42
Compare
squashed |
|
||
// TODO: make setStatefulSetLogVars work | ||
// logging endpoint for stateful sets coming in the future. | ||
var setStatefulSetLogVars = function(set) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we just comment out all of this, no point in running code we arent using
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can do.
return true; | ||
} | ||
// TODO: statefulSets don't have status.phase. As far as I understand, they don't age the | ||
// way other things age, so perhaps this is irrelevant? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep i think you are right, I don't think we would hide any statefulsets ever, so you don't need to change anything when the filter is checked/unchecked
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good, will wipe out most of this fn then.
<small>created <span am-time-ago="set.metadata.creationTimestamp"></span></small> | ||
</div> | ||
<div class="list-group-item-text"> | ||
<!-- TODO: is this the right status? --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably just needs to be the replica counts like we have in the stateful sets table, we dont have a status for stateful sets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. I got this for the other pages, will see if I can work in in here quick as well.
<div class="list-view-pf-additional-info"> | ||
<div class="list-view-pf-additional-info-item"> | ||
<span class="pficon fa-fw pficon-image"></span> | ||
<!-- TODO: not sure what to do with this, gotta figure it out --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like this is right, if you copied it from the dep stuff should be fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ha, sorry, that was an early comment I should have removed :)
</div> | ||
<div ng-repeat-end ng-if="expanded.statefulSets[set.metadata.name]" class="list-group-expanded-section" ng-class="{'expanded': expanded.statefulSets[set.metadata.name]}"> | ||
|
||
<!-- TODO: add logs once we have logs subresource for statefulSets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
honestly i would probably just remove this completely, we dont have logs for replicasets either and it may be awhile before we do, I dont think we left any commented out code for those
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works for me, removed.
|
||
<!-- Until the metrics directive pulls the metrics from the time range of the life of the pod, hide the metrics for old stuff --> | ||
<div class="mar-top-lg" ng-if="metricsAvailable"> | ||
Logs are not available for stateful sets. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesnt seem right, this message only shows up if metricsAvailable is true, you'll still end up with any empty section when metrics arent available
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Um, yeah... that doesn't make sense. Fixed.
- Supports Metrics - Does not currently support logs
875ef42
to
aea91f5
Compare
rebased |
[merge] |
[merge] |
[merge] |
Evaluated for origin web console merge up to aea91f5 |
Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/964/) (Base Commit: 448422b) |
Progress so far:
I think this is doable today, is just a lot of copy/paste & update. No logs (since we don't have the endpoint) but metrics works.
@jwforres @spadgett