-
Notifications
You must be signed in to change notification settings - Fork 192
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
Support more types #57
Conversation
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.
Awesome stuff. Can you add tests that exercise this new code? There's examples in 1 for writing integration and unit tests. It might make sense to split this up for each new API group, especially if you add tests.
]; | ||
options = Object.assign({}, options, { | ||
path: 'apis/apps', | ||
version: options.version || 'v1beta1', |
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.
How should we handle the petset -> statefulset rename 1?
The simplest thing would be to add statefulset
to the genericTypes
array and add a comment about the rename. Any better ideas?
]; | ||
options = Object.assign({}, options, { | ||
path: 'apis/batch', | ||
version: options.version || 'v2alpha1', |
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.
there's a v1
batch -- should we make that the default?
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.
Really good question, I was thinking about that as well.
I guess it makes sense to have the "conservative" default here, anyone that wants alpha/beta stuff should do so knowingly.
class Batch extends ApiGroup { | ||
constructor(options) { | ||
const genericTypes = [ | ||
'scheduledjobs' |
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.
Similar rename issue as above -- scheduledjobs -> cronjobs 1
@@ -5,8 +5,11 @@ module.exports.aliasResources = function (resourceObject) { | |||
// http://kubernetes.io/docs/user-guide/kubectl-overview/ | |||
// and anything else we think is useful. | |||
const resourceAliases = { | |||
clusterroles: [], |
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 we include petsets and scheduledjobs?
@@ -37,7 +37,10 @@ class Namespaces extends BaseObject { | |||
// Generic objects we don't implement special functionality for. | |||
// | |||
const genericTypes = [ | |||
'clusterroles', |
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 we include petsets and scheduledjobs?
@@ -2,9 +2,15 @@ | |||
|
|||
const Core = require('./core'); | |||
const Extensions = require('./extensions'); | |||
const Apps = require('./apps'); | |||
const Batch = require('./batch'); | |||
const RBAC = require('./rbac'); |
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.
Our convention has been to avoid consecutive caps (e.g., apiUrl
, not apiURL
). So this would be Rbac
not RBAC
. Rbac
looks funny, I know, but it follows the convention we're currently using.
This makes it possible to work with apps/v1beta1.StatefulSet instances using the kubernetes-client. This includes support for the deprecated name apps/v1alpha1.PetSet.
This also includes support for the old name 'ScheduledJob'.
…ole,ClusterRoleBinding}
I've fixed the nits above and pushed for a quick look at it - but tests are still missing and coming. This is now rebased this onto #59, while I'm working on the tests :) |
Did you make progress on tests? or get waylaid by that cleanup that needed to happen :) Is there something I can do to help you finish this? |
Relates to #53