Skip to content

[WIP] Add hpa annotations #285

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[WIP] Add hpa annotations #285

wants to merge 2 commits into from

Conversation

benjaminapetersen
Copy link
Contributor

HPA v2beta1 requested from v1 api will have these annotations.

@jwforres @spadgett

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 23, 2018
@spadgett
Copy link
Member

Is it really easier / less risky to read and write these annotations instead just using the v2beta1 API? We could only continue to only support target CPU in our form as long as we don't drop the other values.

@benjaminapetersen
Copy link
Contributor Author

Is a great question. ATM I'm just working out reading them to test for the API & notify the user to go edit the HPA via cli. I think if we wanted to write them, we should def update to the next version instead.

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 23, 2018
@benjaminapetersen benjaminapetersen mentioned this pull request Jan 24, 2018
Merged
benjaminapetersen
benjaminapetersen commented Jan 26, 2018
View reviewed changes
dist/origin-web-common-ui.js
return (icon) ? icon : "fa fa-cube";
};
});
return function(resource, /* optional */ tagName) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All I'm using this for is https://github.com/openshift/origin-web-console/pull/2708/files#diff-04a314c1b6d2d3fc082233f2ee89fa0cR106, reading, not writing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spadgett ping, objections? I'm not setting anything, this is just used to get at the annotations. I could inline a similar fn, but figured this is where it should live.

@benjaminapetersen
Copy link
Contributor Author

I'll close this out per the origin-web-console review comments.

openshift-merge-robot added a commit to openshift/origin-web-console that referenced this pull request Feb 7, 2018
@openshift-merge-robot
Merge pull request #2708 from benjaminapetersen/trello/hpa/v2beta1/ne…
76a44ff 
…w-api-warning

Automatic merge from submit-queue.

hpa v2beta1 api warning on DC & edit autoscaler pages

This PR depends on [common 285](openshift/origin-web-common#285).

[WIP] notes:

`bower link origin-web-common` with the above PR or the tests will fail. 

On the DC page:

![screen shot 2018-01-23 at 6 21 58 pm](https://user-images.githubusercontent.com/280512/35306332-132de372-006b-11e8-8552-ee94bd4673a2.png)

- probably only want to show the v2beta1 message, will hide the other

On the edit autoscaler page:

![screen shot 2018-01-23 at 6 25 12 pm](https://user-images.githubusercontent.com/280512/35306333-1337b924-006b-11e8-9cc7-3abacf3a6e91.png)

- shows a message w/a link to CLI
- hides the CPU request input as editing this is probably not what the user wants to do

TODO (perhaps follow-on PR):

- Update the warning message to show the user the data from the `metrics` annotation so they can easily see what the autoscaler is using to scale the resource.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@benjaminapetersen @spadgett @openshift-ci-robot