-
Notifications
You must be signed in to change notification settings - Fork 38.6k
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
Web UI v1beta3 conversion #8304
Conversation
Hrm, there are lots of API level chances in switching. Have we validated that the javascript code can handle it? |
@brendanburns I'm going to focus on the API level changes as well. We'll make sure the javascript code is modified as well. I apologize if that wasn't clearly communicated. |
Yes, I pointed out some of the problems in another PR: #8302 (comment) |
We'll also need to update the configuration collector. |
Yeah, I expect a bunch of js changes before this will work. (note if you tried it like this it probably worked fine because the bin data didn't get regenerated) |
557e70a
to
170f517
Compare
I'm splitting this PR up into many smaller ones for easier review. |
170f517
to
8089fb1
Compare
"resourceVersion": 166552, | ||
"apiVersion": "v1beta1", | ||
"apiVersion": "v1beta3", |
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.
As in the other PR, all of these examples are still using the v1beta1 schema. They need to be totally regenerated using v1beta3.
8089fb1
to
e423bfc
Compare
@bgrant0607 I generated new mock files and updated the code. I used on the conversion utility on the earlier commit, which apparently does not account for selfLink. Thanks for the comments. |
All pages are updated and ready for review. Once these are all in I will do followup PR's removing the v1beta2 api, and also updating to the v1 api endpoint. |
ping one this one since a lot of prs depending on it. |
@@ -205,7 +206,7 @@ app.controller('TabCtrl', [ | |||
.service('cAdvisorService', ["$http", "$q", "ENV", function($http, $q, ENV) { | |||
var _baseUrl = function(minionIp) { | |||
var minionPort = ENV['/']['cAdvisorPort'] || "8081"; | |||
var proxy = ENV['/']['cAdvisorProxy'] || "/api/v1beta2/proxy/nodes/"; | |||
var proxy = ENV['/']['cAdvisorProxy'] || "/api/v1beta3/proxy/nodes/"; |
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.
Should it be /api/v1beta3/proxy/namespaces/default/nodes?
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.
/nodes isn't namespaced and /api/v1beta3/proxy/namespaces/default/nodes throws a 404, so leaving it as is
e423bfc
to
a3fd6c0
Compare
a3fd6c0
to
cda66d9
Compare
Update mocks to v1beta3
cda66d9
to
90d22c4
Compare
Thanks @caesarxuchao I've made the setNamespace change, PTAL |
Thanks, @bcbroussard. I'm not familiar with .js, so I just checked the v1beta3 syntax, LGTM. |
Foundation for v1beta3 api upgrade of web ui
cc/ @lavalamp @bgrant0607 @jackgr