[WIP] PetSets / StatefulSets #1088

benjaminapetersen · 2017-01-06T19:44:43Z

Initial run at PetSets/StatefulSets

Currently supports:

Edit YAML
Delete
Readonly env vars (cannot edit env vars on a PetSet)
Annotations
Labels

Screenshots:

I have a running list of about 15 more tasks, but wanted to get this up here sooner than later & see if we can merge in PetSets in stages.

@jwforres @spadgett

jwforres

can you change all references to petset to statefulset, also as soon as the rebase lands we will need to switch to requests statefulsets instead of petsets

benjaminapetersen · 2017-01-09T20:03:40Z

Forgot to include this screenshot. Need to decide, do we want these on the deployments page, or do they get a dedicated (and different table list data)?

benjaminapetersen · 2017-01-10T15:29:04Z

Added:

Metrics
Events
Storage
Updated PetSet to StatefulSet (despite the hello-petset name)

jwforres · 2017-01-10T15:39:08Z

@benjaminapetersen did you push those changes?

jwforres · 2017-01-10T15:40:04Z

also breadcrumb in your screen shot still says Pet Sets

benjaminapetersen · 2017-01-10T19:23:44Z

Working with the table list to figure out what is most meaningful. Trigger needs to be removed yet & replaced with something else.

benjaminapetersen · 2017-01-10T19:24:46Z

@jwforres updates pushed if you want to start taking a look.

jwforres · 2017-01-10T19:29:16Z

app/scripts/app.js

@@ -174,6 +174,11 @@ angular
        templateUrl: 'views/edit/deployment-config.html',
        controller: 'EditDeploymentConfigController'
      })
+      .when('/project/:project/browse/stateful-sets/:statefulset', {


capital S on Set in the param :statefulSet

Done, though the annoying thing about camelCase here is that it doesn't work in the controller when using $routeParams. It will still be $routeParams.statefulset :(

jwforres · 2017-01-10T19:29:24Z

app/scripts/app.js

@@ -404,6 +409,9 @@ angular
      .when('/project/:project/browse/deployments-replicationcontrollers/:rc', {
        redirectTo: '/project/:project/browse/rc/:rc'
      })
+      .when('/project/:project/browse/pet-sets/:petset', {
+        redirectTo: '/project/:project/browse/stateful-sets/:statefulset'


why is this needed

Not anymore, but initially i was thinking we would get this in to support PetSets, then update to StatefulSets. Will remove.

jwforres · 2017-01-10T19:29:54Z

app/scripts/controllers/deployments.js

+          group: 'apps',
+          version: 'v1alpha1'
+        }, context, function(statefulSets) {
+          console.log('statefulSets?', statefulSets.by('metadata.name'));


remove console.log

Will do. Assumed this block is tmp code till we agree it will land.

jwforres · 2017-01-10T19:30:58Z

app/scripts/controllers/statefulSet.js

@@ -0,0 +1,90 @@
+'use strict';
+
+// TODO: statefulSetController rename


you can remove this TODO now right?

jwforres · 2017-01-10T19:32:46Z

app/scripts/controllers/statefulSet.js

+    ProjectsService) {
+
+    $scope.projectName = $routeParams.project;
+    $scope.statefulSetName = $routeParams.statefulset || $routeParams.petset; // TODO: eliminate petset fallback


will want to make sure we remove this before merging, why is this currently needed?

Per the above, was assuming we may merge & update.

jwforres · 2017-01-10T20:09:18Z

app/views/browse/stateful-set.html

+                  <uib-tab-heading>Events</uib-tab-heading>
+                  <div class="resource-events">
+                      <events
+                        resource-kind="PetSet"


TODO here to switch to stateful set

jwforres · 2017-01-10T20:10:43Z

app/views/deployments.html

+                <thead>
+                  <tr>
+                    <th>Name</th>
+                    <th>Generation</th>


a lot of objects have Generation and we don't usually show it, I would probably just go with Name, Status, Created for now, we can discuss other options during design review thurs.

Sounds good.

jwforres · 2017-01-10T20:11:46Z

app/views/deployments.html

+                    <td data-title="Status">
+                      <status-icon status="statefulSet | deploymentStatus"></status-icon>
+                      {{statefulSet | deploymentStatus}}
+                      <span ng-if="(statefulSet | deploymentStatus) == 'Active' || (statefulSet | deploymentStatus) == 'Running'">,


i'm sure this was copied but these should really be triple equals ===

jwforres · 2017-01-10T20:12:03Z

app/views/deployments.html

+                      {{statefulSet | deploymentStatus}}
+                      <span ng-if="(statefulSet | deploymentStatus) == 'Active' || (statefulSet | deploymentStatus) == 'Running'">,
+                        <span ng-if="statefulSet.spec.replicas !== statefulSet.status.replicas">{{statefulSet.status.replicas}}/</span>
+                        {{statefulSet.spec.replicas}} replica<span ng-if="statefulSet.spec.replicas != 1">s</span>


jwforres · 2017-01-10T20:13:18Z

app/views/deployments.html

@@ -77,8 +116,11 @@ <h3 ng-if="(deployments | hashSize) || (replicaSets | hashSize)">Deployment Conf
                      <div row class="status">
                        <status-icon status="replicationController | deploymentStatus" disable-animation></status-icon>
                        <span flex>
-                          {{replicationController | deploymentStatus}}<span ng-if="(replicationController | deploymentStatus) == 'Active' || (replicationController | deploymentStatus) == 'Running'">,
-                          <span ng-if="replicationController.spec.replicas !== replicationController.status.replicas">{{replicationController.status.replicas}}/</span>{{replicationController.spec.replicas}} replica<span ng-if="replicationController.spec.replicas != 1">s</span></span>
+                          {{replicationController | deploymentStatus}}


you have introduced whitespace here that will cause the , to have a space in front of it

bah, html. 😒

dont forget you need to fix the fact that this is adding whitespace

benjaminapetersen · 2017-01-12T21:34:50Z

Ok, updated comments above. I think the only outstanding is:

Where do these live (I'm still leaning towards a dedicated page)
If we do/don't update to a dedicated page, how do we want the breadcrumbs done (prob Deployments >> StatefulSetName if not Stateful Sets >> StatefulSetName)

Working on adding the donut now.

benjaminapetersen · 2017-01-17T19:58:43Z

New screenshots, pushing code soon:

jwforres · 2017-01-17T20:35:11Z

the empty Storage section looks odd, if there is nothing under storage it should at least have some text that says that. Related, did you add the section for showing the volumes defined in the pod spec vs showing the volumeClaimTemplate?

…

On Tue, Jan 17, 2017 at 2:58 PM, Ben Petersen ***@***.***> wrote: New screenshots, pushing code soon: [image: screen shot 2017-01-17 at 2 57 54 pm] <https://cloud.githubusercontent.com/assets/280512/22037511/659ee216-dcc5-11e6-9937-794d3cee8412.png> [image: screen shot 2017-01-17 at 2 55 12 pm] <https://cloud.githubusercontent.com/assets/280512/22037467/3c41296a-dcc5-11e6-8964-09cfbd041592.png> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1088 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABZk7XLQR2yOkz9FRINpsExZL7yOKeh3ks5rTR10gaJpZM4LdCiI> .

benjaminapetersen · 2017-01-17T21:12:44Z

@jwforres tinkering with the storage, including:

remove volume
add storage
add config files

Add Storage gives me an error, but I'm diagnosing (I dont think this came from the API):

jwforres · 2017-01-17T21:21:12Z

no @spadgett has checks on those pages that restricts what kinds are allowed

…

On Tue, Jan 17, 2017 at 4:12 PM, Ben Petersen ***@***.***> wrote: @jwforres <https://github.com/jwforres> tinkering with the storage, including: - remove volume - add storage - add config files Add Storage gives me an error, but I'm diagnosing (I dont think this came from the API): [image: screen shot 2017-01-17 at 4 11 20 pm] <https://cloud.githubusercontent.com/assets/280512/22040091/aa77af44-dccf-11e6-917a-9b1338cc36dc.png> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1088 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABZk7VXV__iJB0HN883s8k7UKNZUJ0BKks5rTS7NgaJpZM4LdCiI> .

benjaminapetersen · 2017-01-17T21:25:19Z

Heh, yeah all of those actions are forbidden. neverthemind :)

benjaminapetersen · 2017-01-17T21:26:01Z

Right, I added it to the allowed filter in the controllers, but the API response is still forbidden via a 422 error.

benjaminapetersen · 2017-01-18T03:40:51Z

Perhaps something like this for persistent volume claim templates?

Deliberately looks like volumes, but I'm trying to decide if its confusing.

jwforres · 2017-01-18T14:33:54Z

Hmm, so we have largely stopped saying PVC in a lot of places, I'm wondering if this should say something like "Storage Template" or "Provisioned Storage Template" and put that section right underneath the Template section, and then maybe Volumes should be called Attached Volumes instead?

@spadgett thoughts?

jwforres · 2017-01-18T14:35:06Z

Either way it probably needs a help tip or something next to the storage template section that explains what it is, since its a new concept that only exists on stateful sets. Or if we are able to get a subsection in the new stateful set docs page that talks about it we could link to that

benjaminapetersen · 2017-01-18T14:39:30Z

@adellape ping! The above comment might be helpful for something we could use in the new docs page.

benjaminapetersen · 2017-01-18T15:46:15Z

I do prefer it closer to the template above, wonder if I should mimic the style (grey left border, icons(?), etc):

jwforres · 2017-01-18T15:57:56Z

Its probably worth trying, but I'm not sure what icon you would use for Access Mode. Also access mode needs to be humanized.

benjaminapetersen · 2017-01-18T16:04:07Z

I think I do like the direction of this better:

Prob we can iron out the icons. The disk is so-so. The "server" icon seems too small to see & not sure its exactly right.

jwforres · 2017-01-18T16:07:59Z

the server icon should probably be the database icon instead, like the mount icon in the pod template

benjaminapetersen · 2017-01-18T17:11:50Z

Updated to the database icon, couple other options for the access mode icon:

Disk icon for access mode:

Lock icon for access mode:

Key icon for access mode:

@ajacobs21e & (does liz have a github id?)

benjaminapetersen · 2017-01-18T18:24:14Z

@lizsurette re: the icons

lizsurette · 2017-01-18T18:32:27Z

@benjaminapetersen I think the lock icon is fairly standard for representing something that deals with "permissions". Do you guys use that icon anywhere else? I just wouldn't want it to be used differently in two places and confuse anyone. @ajacobs21e

jwforres · 2017-01-18T20:31:38Z

app/scripts/services/navigate.js

@@ -196,6 +196,12 @@ angular.module("openshiftConsole")
              .segmentCoded(name.substring(0, ind))
              .segmentCoded(name.substring(ind + 1));
            break;
+          // PetSet & StatefulSet handle the same
+          case "PetSet" :


we can remove this now correct?

jwforres · 2017-01-18T20:33:41Z

app/views/_volume-claim-templates.html

+  <div
+    ng-repeat="template in templates"
+    class="pod-template">
+    <div class="component-label">Claim: {{template.metadata.name}}</div>


Storage Claim:

jwforres · 2017-01-18T20:33:55Z

app/views/_volume-claim-templates.html

+        <span class="fa fa-database" aria-hidden="true"></span>
+      </div>
+      <div flex class="word-break">
+        <span class="pod-template-key">Storage:</span>


jwforres · 2017-01-18T20:34:19Z

app/views/_volume-claim-templates.html

+      <div flex class="word-break">
+        <span class="pod-template-key">Access Modes:</span>
+        <span ng-repeat="mode in template.spec.accessModes">
+          {{mode}}


if its not $last add a ', '

jwforres · 2017-01-18T20:36:22Z

app/views/deployments.html

          <div class="row">
            <div class="col-md-12">
+              <alerts alerts="alerts"></alerts>


assume this was moved intentionally?

I just reset this file since im not working on this page anymore.

jwforres · 2017-01-18T20:37:44Z

app/views/deployments.html

@@ -77,8 +116,11 @@ <h3 ng-if="(deployments | hashSize) || (replicaSets | hashSize)">Deployment Conf
                      <div row class="status">
                        <status-icon status="replicationController | deploymentStatus" disable-animation></status-icon>
                        <span flex>
-                          {{replicationController | deploymentStatus}}<span ng-if="(replicationController | deploymentStatus) == 'Active' || (replicationController | deploymentStatus) == 'Running'">,
-                          <span ng-if="replicationController.spec.replicas !== replicationController.status.replicas">{{replicationController.status.replicas}}/</span>{{replicationController.spec.replicas}} replica<span ng-if="replicationController.spec.replicas != 1">s</span></span>
+                          {{replicationController | deploymentStatus}}


dont forget you need to fix the fact that this is adding whitespace

benjaminapetersen · 2017-01-18T21:28:19Z

Quick screenshot update, "selector" used a label icon:

jwforres · 2017-01-18T21:46:40Z

LGTM, clean up the PR and let me know when you are ready for final review

…

On Wed, Jan 18, 2017 at 4:28 PM, Ben Petersen ***@***.***> wrote: Quick screenshot update, "selector" used a label icon: [image: screen shot 2017-01-18 at 4 17 56 pm] <https://cloud.githubusercontent.com/assets/280512/22083660/14db3d76-dd9b-11e6-8044-1403679682be.png> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1088 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABZk7fcxb46q4jKELS7_vYTwd3gHfhmhks5rToP0gaJpZM4LdCiI> .

benjaminapetersen · 2017-01-18T22:06:00Z

@jwforres updated & squashed

- Details, Environment, Metrics, Events - Edit YAML - Delete - Pods list - Annotations - Template & volume claim template - No support currently for scale/autoscale - hopefully API support in near future

jwforres · 2017-01-18T22:13:39Z

app/views/_volume-claim-templates.html

+
+    <div
+      row class="pod-template-image icon-row"
+      ng-if="template.spec.selector.matchLabels">


ok looks like this is using the new type of selectors, we can do a follow-up PR tomorrow, but we should handle matchExpressions as well, can you make a TODO for that for tomorrow in your notebook :)

@benjaminapetersen We have a selector directive now.

<selector selector="template.spec.selector"></selector>

Oh nice, I'll update this

Hmmm. So if I swap that in I get this:

@sg00dwin I think the css in the pod-template targeting div:first-child is probably what is doing it, probably too greedy:

.icon-row div:first-child { text-align: center; width: 30px; }

Pinging you before I update as this is used in a bunch of places.

jwforres · 2017-01-18T22:14:44Z

[merge]

openshift-bot · 2017-01-18T22:15:24Z

Evaluated for origin web console merge up to 075d122

openshift-bot · 2017-01-18T22:23:53Z

Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/935/) (Base Commit: 2cb38e1)

jwforres · 2017-01-19T01:09:02Z

yeah... that would fix it :)

…

On Wed, Jan 18, 2017 at 8:05 PM, Sam Padgett ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In app/views/_volume-claim-templates.html <#1088>: > + + <div row class="pod-template-image icon-row"> + <div> + <span class="fa fa-database" aria-hidden="true"></span> + </div> + <div flex class="word-break"> + <span class="pod-template-key">Capacity:</span> + <span> + {{template.spec.resources.requests.storage}} + </span> + </div> + </div> + + <div + row class="pod-template-image icon-row" + ng-if="template.spec.selector.matchLabels"> @benjaminapetersen <https://github.com/benjaminapetersen> We have a selector directive now. <selector selector="template.spec.selector"></selector> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1088>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABZk7chdB03Fd2z15gVmfmGs52LFo4HOks5rTrbKgaJpZM4LdCiI> .

ajacobs21e · 2017-01-19T14:16:09Z

@benjaminapetersen I didn't have a chance to read through all this until now. I agree with Liz, if we are using the lock elsewhere then I wouldn't use it here.

Should readonly be read only or read-only? It looks funny to me as one word. Is it a common term to write it that way?

jwforres reviewed Jan 6, 2017

View reviewed changes

benjaminapetersen changed the title ~~[WIP] Petsets statefulsets~~ [WIP] PetSets / StatefulSets Jan 6, 2017

jwforres requested changes Jan 10, 2017

View reviewed changes

benjaminapetersen force-pushed the petsets-statefulsets branch from 851a739 to ed67197 Compare January 12, 2017 21:34

benjaminapetersen force-pushed the petsets-statefulsets branch from ed67197 to 7750476 Compare January 17, 2017 20:47

benjaminapetersen force-pushed the petsets-statefulsets branch 2 times, most recently from 0cc34ab to b28dc2b Compare January 18, 2017 17:22

jwforres requested changes Jan 18, 2017

View reviewed changes

benjaminapetersen force-pushed the petsets-statefulsets branch from b28dc2b to fa22ce7 Compare January 18, 2017 22:04

Add StatefulSets browse page

075d122

- Details, Environment, Metrics, Events - Edit YAML - Delete - Pods list - Annotations - Template & volume claim template - No support currently for scale/autoscale - hopefully API support in near future

benjaminapetersen force-pushed the petsets-statefulsets branch from fa22ce7 to 075d122 Compare January 18, 2017 22:07

jwforres reviewed Jan 18, 2017

View reviewed changes

jwforres approved these changes Jan 18, 2017

View reviewed changes

openshift-bot merged commit f6709f6 into openshift:master Jan 18, 2017

benjaminapetersen deleted the petsets-statefulsets branch January 18, 2017 22:50

benjaminapetersen mentioned this pull request Jan 19, 2017

[WIP] stateful sets updated volume claim template selector #1144

Closed

		@@ -0,0 +1,90 @@
		'use strict';

		// TODO: statefulSetController rename

[WIP] PetSets / StatefulSets #1088

[WIP] PetSets / StatefulSets #1088

Conversation

benjaminapetersen commented Jan 6, 2017

jwforres left a comment

Choose a reason for hiding this comment

benjaminapetersen commented Jan 9, 2017

benjaminapetersen commented Jan 10, 2017

jwforres commented Jan 10, 2017

jwforres commented Jan 10, 2017

benjaminapetersen commented Jan 10, 2017

benjaminapetersen commented Jan 10, 2017

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

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

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

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

Choose a reason for hiding this comment

benjaminapetersen commented Jan 12, 2017

benjaminapetersen commented Jan 17, 2017

jwforres commented Jan 17, 2017 via email

benjaminapetersen commented Jan 17, 2017

jwforres commented Jan 17, 2017 via email

benjaminapetersen commented Jan 17, 2017

benjaminapetersen commented Jan 17, 2017 • edited Loading

benjaminapetersen commented Jan 18, 2017

jwforres commented Jan 18, 2017

jwforres commented Jan 18, 2017

benjaminapetersen commented Jan 18, 2017

benjaminapetersen commented Jan 18, 2017

jwforres commented Jan 18, 2017

benjaminapetersen commented Jan 18, 2017

jwforres commented Jan 18, 2017

benjaminapetersen commented Jan 18, 2017

benjaminapetersen commented Jan 18, 2017

lizsurette commented Jan 18, 2017

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

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

benjaminapetersen commented Jan 18, 2017

jwforres commented Jan 18, 2017 via email

benjaminapetersen commented Jan 18, 2017

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

jwforres commented Jan 18, 2017

openshift-bot commented Jan 18, 2017

openshift-bot commented Jan 18, 2017 • edited Loading

jwforres commented Jan 19, 2017 via email

ajacobs21e commented Jan 19, 2017

benjaminapetersen commented Jan 17, 2017 •

edited

Loading

openshift-bot commented Jan 18, 2017 •

edited

Loading