Add Agent platform type under NodePool cmd #956
Conversation
|
/lgtm |
cmd/nodepool/none/create.go
Outdated
| @@ -0,0 +1,44 @@ | |||
| package none | |||
There was a problem hiding this comment.
I don’t think is clear just yet how we'll angle None. We could communicate that when platform None we support NodePool to generate an ign config for you and enable at the cli, though that feels weird since that NodePool is not scalable nor have any capability.
Please drop it for now until we agree that we need it.
There was a problem hiding this comment.
Dropped None platform
cmd/nodepool/core/create.go
Outdated
| Type() hyperv1.PlatformType | ||
| } | ||
|
|
||
| func (o *CreateNodePoolOptions) CreateExecFunc(platformOpts PlatformOptions) func(cmd *cobra.Command, args []string) error { |
There was a problem hiding this comment.
There was a problem hiding this comment.
I just moved existing (duplicated) code from the platforms files and wrote it once, to be used from all platforms
What is the exacly issue here?
There was a problem hiding this comment.
I'm just suggesting to use a single term in naming and replace "exec" by "run" as it seems to be how the owning package refers to them https://github.com/spf13/cobra/blob/e04ec725508c760e70263b031e5697c232d5c3fa/command.go#L98
cmd.RunE
nirarg
force-pushed
the
nodepool-platforms
branch
from
41d13a1 to
9fede92
Compare
|
/retitle Add Agent platform type under NodePool cmd |
openshift-ci
bot
changed the title
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enxebre, nirarg The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
Since changed the NodePool to be platform aware (PR openshift#832) In order to create NodePool for specific platform, the platform must implement specific sub command Therfore add the subcommands for Agent platform, although the NodePool platformSpec is empty for those platforms
nirarg
force-pushed
the
nodepool-platforms
branch
from
9fede92 to
d939a50
Compare
|
@nirarg: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
5 participants
What this PR does / why we need it:
Since changed the NodePool to be platform aware (PR #832)
In order to create NodePool for specific platform, the platform must implement specific sub command
Therfore add the subcommands Agent platform, although the NodePool platformSpec is empty for those platforms
Checklist