-
Notifications
You must be signed in to change notification settings - Fork 79
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 for network and ip allocation #1
Conversation
@jkraj please review |
GNUmakefile
Outdated
@sh -c "'$(CURDIR)/scripts/errcheck.sh'" | ||
|
||
vendor-status: | ||
@govendor status |
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.
we are not using govendor so remove it
infoblox/provider.go
Outdated
Type: schema.TypeString, | ||
Optional: true, | ||
DefaultFunc: schema.EnvDefaultFunc("WAPI_VERSION", "2.8"), | ||
Description: "which wapi version ro be used", |
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.
write validate function for all supporter wapi versions.
- Have a list of wapi_version constants
- Check the user passed version available in the list.
infoblox/provider.go
Outdated
func Provider() terraform.ResourceProvider { | ||
return &schema.Provider{ | ||
Schema: map[string]*schema.Schema{ | ||
"host": &schema.Schema{ |
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.
infoblox/resourceAllocation.go
Outdated
Delete: resourceAllocationRelease, | ||
|
||
Schema: map[string]*schema.Schema{ | ||
"networkviewname": &schema.Schema{ |
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.
network_view_name
infoblox/resourceAllocation.go
Outdated
macaddr := d.Get("macaddr").(string) | ||
vmID := d.Get("vmid").(string) | ||
connector := m.(*ibclient.Connector) | ||
objMgr := ibclient.NewObjectManager(connector, "terraform", "goclient1") |
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 don't want this line in all the methods 'objMgr := ibclient.NewObjectManager(connector, "terraform", "goclient1")' try to optimize it.
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.
'goclient1' should be replaced by the 'tenant id' which should be provided by user, include this as option parameter in all resource and default value as null.
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.
changed at all places
infoblox/resourceNetworkView.go
Outdated
Delete: resourceNetworkViewDelete, | ||
|
||
Schema: map[string]*schema.Schema{ | ||
"networkviewname": &schema.Schema{ |
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.
network_view_name
infoblox/resourceNetworkView.go
Outdated
return nil | ||
} | ||
func resourceNetworkViewDelete(d *schema.ResourceData, m interface{}) 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.
mention why it is not implemented.
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 will put that in the read me
func main() { | ||
plugin.Serve(&plugin.ServeOpts{ | ||
ProviderFunc: infoblox.Provider }) | ||
} |
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.
add a new line at the end.
infoblox/provider.go
Outdated
Type: schema.TypeString, | ||
Required: true, | ||
DefaultFunc: schema.EnvDefaultFunc("HOST", nil), | ||
Description: "NIOS IP address", | ||
Description: "NIOS IP address. Server IP address", |
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.
NIOS IP address. Server IP address. Why twice. "NIOS Server IP Address"
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.
added
infoblox/provider.go
Outdated
}, | ||
"username": &schema.Schema{ | ||
Type: schema.TypeString, | ||
Required: true, | ||
DefaultFunc: schema.EnvDefaultFunc("USERNAME", nil), | ||
Description: "Grid manager username", | ||
Description: "User to authenticate ", |
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.
Description looks like incomplete.
please the description and variable as it is from here. https://github.com/sky-uk/terraform-provider-infoblox/blob/master/infoblox/provider.go
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.
placed the descriptions which ever related to us
infoblox/resourceNetwork.go
Outdated
Type: schema.TypeString, | ||
Required: true, | ||
DefaultFunc: schema.EnvDefaultFunc("net_view_name", nil), | ||
Description: "give the nnetviewname you created", | ||
Description: "give the net_view_name you created", |
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.
why description having variable name?
Start the sentence with caps
Read the sentence does it make sense?
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.
changed
infoblox/resourceNetwork.go
Outdated
Type: schema.TypeString, | ||
Required: true, | ||
DefaultFunc: schema.EnvDefaultFunc("net_address", nil), | ||
Description: "", |
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.
What happens to the descriptions
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.
added
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.
infoblox/provider.go
Outdated
Type: schema.TypeInt, | ||
Optional: true, | ||
DefaultFunc: schema.EnvDefaultFunc("POOL_CONNECTIONS", "10"), | ||
Description: "Maximum number of connections to establish to the database. Zero means unlimited.", |
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.
This is not database.
infoblox/provider.go
Outdated
Type: schema.TypeString, | ||
Optional: true, | ||
DefaultFunc: schema.EnvDefaultFunc("PORT", "443"), | ||
Description: "Port number used for connection", |
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.
add Infoblox appliance
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.
We decided to use Server instead of appliances.
infoblox/provider.go
Outdated
Type: schema.TypeString, | ||
Required: true, | ||
DefaultFunc: schema.EnvDefaultFunc("USERNAME", nil), | ||
Description: "User to authenticate with Infoblox appliance ", |
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.
remove space
infoblox/provider.go
Outdated
Type: schema.TypeString, | ||
Optional: true, | ||
DefaultFunc: schema.EnvDefaultFunc("WAPI_VERSION", "2.8"), | ||
Description: "Infoblox WAPI server version, defaults to v2.8", |
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.
WAPI version of Infoblox Appliance
infoblox/resourceNetwork.go
Outdated
Type: schema.TypeString, | ||
Required: true, | ||
DefaultFunc: schema.EnvDefaultFunc("network_view_name", nil), | ||
Description: "Give the network view name you created", |
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.
- remove extra space after name
- Network view name available in NIOS Appliance ( change the description )
- Make it optional
- specfiy 'default' in DefaultFunc
infoblox/resourceNetwork.go
Outdated
Type: schema.TypeString, | ||
Required: true, | ||
DefaultFunc: schema.EnvDefaultFunc("network_name", nil), | ||
Description: "The name you want to give to your network", |
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.
The name of the Network
Type: schema.TypeString, | ||
Required: true, | ||
DefaultFunc: schema.EnvDefaultFunc("net_address", nil), | ||
Description: "Give the address in cidr format", |
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.
"network": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Description: "The network address in IPv4 Address/CIDR format.",
},
infoblox/resourceNetwork.go
Outdated
Type: schema.TypeString, | ||
Optional:true, | ||
DefaultFunc: schema.EnvDefaultFunc("tennant_id",nil), | ||
Description:"Unique identifier of your instance", |
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.
Unique identifier of your instance in the cloud
infoblox/resourceNetworkView.go
Outdated
DefaultFunc: schema.EnvDefaultFunc("network_view_name", nil), | ||
Description: "The name you want to give to your network view", | ||
}, | ||
"tennant_id": &schema.Schema{ |
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.
tenant not tennant
this section have format issue.
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.
Where is the README.md with basic steps like to compile/build etc
infoblox/provider.go
Outdated
ResourcesMap: map[string]*schema.Resource{ | ||
"infoblox_network": resourceNetwork(), | ||
"infoblox_network_view": resourceNetworkView(), | ||
"infoblox_ip_allocation": resourceAllocation(), |
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.
Change resourceAllocation to resoureIPAddress()
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.
change based on my review comments
scripts/gogetcookie.sh
Outdated
@@ -0,0 +1,10 @@ | |||
#!/bin/bash |
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.
Remove this file
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.
Once the go vet passed submit the changes
go vet runs
i will prepare the ppt and then figure out a way to optimize.
No description provided.