-
Notifications
You must be signed in to change notification settings - Fork 30
Add generic_config to node template resource #75
base: master
Are you sure you want to change the base?
Add generic_config to node template resource #75
Conversation
7a4c069
to
1e31d02
Compare
|
1e31d02
to
d0f9c4a
Compare
|
Am I correct in understanding that this will lay the groundwork for a vSphere node template? but not the actual vSphere node template? Can someone from Rancher comment on when vSphere support is expected? |
@yamamoto-febc Can you take resolve the merge conflict? @ernst-at-tv2 I'll look into that. |
This is a step in the right direction but I was wondering if it would be possible to keep some sort of type validation in the configuration? As it is proposed right now, there would be no validation and if a field is wrong (ie wrong type) this would only be reflected at node creation time. Would it be possible to maybe add a schema resource to do type validation? Or do you think that we could maybe write our own provider that could be the input for the configuration? |
@yamamoto-febc, thanks for your PR and sorry for the delay. @ernst-at-tv2, vsphere cloud credential and node template support is added on PR #91 @rodcloutier, the scope of generic driver is to be able to add |
d0f9c4a
to
e749486
Compare
I have just rebased on latest master. This update includes followings:
|
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.
@yamamoto-febc, could you please take a look to changes requested?? Take special care reviewing driverID and driverName assignments and use.
@@ -14,6 +17,89 @@ type CloudCredential struct { | |||
DigitaloceanCredentialConfig *digitaloceanCredentialConfig `json:"digitaloceancredentialConfig,omitempty" yaml:"digitaloceancredentialConfig,omitempty"` | |||
OpenstackCredentialConfig *openstackCredentialConfig `json:"openstackcredentialConfig,omitempty" yaml:"openstackcredentialConfig,omitempty"` | |||
VmwarevsphereCredentialConfig *vmwarevsphereCredentialConfig `json:"vmwarevspherecredentialConfig,omitempty" yaml:"vmwarevspherecredentialConfig,omitempty"` | |||
genericCredentialConfig *genericCredentialConfig |
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.
Could you please make genericCredentialConfig
field public and add json and yaml definition??
- genericCredentialConfig *genericCredentialConfig
+ GenericCredentialConfig *genericCredentialConfig `json:"genericcredentialConfig,omitempty" yaml:"genericcredentialConfig,omitempty"`
return "" | ||
} | ||
|
||
func (n *CloudCredential) UnmarshalJSON(data []byte) error { |
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.
I didn't find it, but is UnmarshalJSON
function used anywhere??
return nil | ||
} | ||
|
||
func (n *CloudCredential) MarshalJSON() ([]byte, error) { |
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.
I didn't find it, but is MarshalJSON
function used anywhere??
driverName == vmwarevsphereConfigDriver | ||
} | ||
|
||
func (n *NodeTemplate) UnmarshalJSON(data []byte) error { |
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.
I didn't find it, but is UnmarshalJSON
function used anywhere??
return nil | ||
} | ||
|
||
func (n *NodeTemplate) MarshalJSON() ([]byte, error) { |
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.
I didn't find it, but is MarshalJSON function used anywhere??
} | ||
|
||
return s | ||
} |
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.
genericCredentialConfig
has 3 fields defined, but just 2 fields defined on cloudCredentialGenericFields
function.
Do you really need driverName
?? If it's needed, please define it as field on cloudCredentialGenericFields
function.
@@ -14,6 +17,72 @@ type NodeTemplate struct { | |||
DigitaloceanConfig *digitaloceanConfig `json:"digitaloceanConfig,omitempty" yaml:"digitaloceanConfig,omitempty"` | |||
OpenstackConfig *openstackConfig `json:"openstackConfig,omitempty" yaml:"openstackConfig,omitempty"` | |||
VmwarevsphereConfig *vmwarevsphereConfig `json:"vmwarevsphereConfig,omitempty" yaml:"vmwarevsphereConfig,omitempty"` | |||
genericConfig *genericNodeTemplateConfig |
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.
Could you please make genericConfig
field public and add json and yaml definition??
- genericConfig * genericNodeTemplateConfig
+ GenericConfig * genericNodeTemplateConfig `json:"genericConfig,omitempty" yaml:"genericConfig,omitempty"`
genericConfig *genericNodeTemplateConfig | ||
} | ||
|
||
func isTypedNodeDriver(driverName string) bool { |
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.
Just to clarify, amazonec2ConfigDriver
and others are driver ID's not names.
return nil, fmt.Errorf("[ERROR] Driver %s can not be used with generic_credential_config", gc.driverID) | ||
} | ||
obj.genericCredentialConfig = gc | ||
in.Set("driver", obj.genericCredentialConfig.driverName) |
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.
driver
should be set as driverID not driverName
return nil, fmt.Errorf("[ERROR] Node template driver %s can not be used with generic_config", gc.driverID) | ||
} | ||
obj.genericConfig = gc | ||
obj.Driver = obj.genericConfig.driverName |
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.
obj.Driver
should be set as driverID not driverName
Due to rancher2 terraform provider is already officially supported by Hashicorp, this repo is going to be archived in favour of https://github.com/terraform-providers/terraform-provider-rancher2 Are you still interested in this PR?? |
This PR adds
generic_config
to node template resource.This allows driver specific parameters to be specified as key/value pairs to support various node drivers in the node template.
Motivation
Rancher is supporting many node drivers.
However, this provider only supports
amazonec2
,azure
, anddigitalocean
now.It is extendable by implements driver specific codes(e.g.: for vsphere), but it limits Rancher's flexibility.
This PR opens the door to enable various node drivers to be used without driver specific codes.
Implementation
generic_config
to schema of node template resource.generic_config
are held in NodeTemplate struct.MarshalJSON/UnmarshalJSON
to communicate driver specific parameters with the Rancher API.Example usage of
generic_config
With builtin driver
With custom driver