Skip to content
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

Provider Oneandone #13633

Merged
merged 7 commits into from
Apr 21, 2017
Merged

Provider Oneandone #13633

merged 7 commits into from
Apr 21, 2017

Conversation

jasmingacic
Copy link

This is a provider for 1&1.
Cloud Server API documentation is here

This provider manages following 1&1 resources:

  • Server
  • Firewall Policy
  • Monitoring Policy
  • Shared Storage
  • Load Balancer
  • Private Networks
  • VPN
  • Public IP

For credentials for acceptance tests please contact me on via email.

Copy link
Contributor

@stack72 stack72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jasminSPC

Ran through the first couple of resources and found the same issues in both

Can you have a look and see if you can roll the same changes across all of the resources?

Thanks

Paul

Optional: true,
Default: 50,
},
"endpoint": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to be able to allow this to be set via ENV VAR as well

"retries": {
Type: schema.TypeInt,
Optional: true,
Default: 50,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also ENV VAR would be a good addition

}
config := Config{
Token: d.Get("token").(string),
Retries: d.Get("retries").(int),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retries are passed to the Config struct but not actually used in there?

Copy link
Author

@jasmingacic jasmingacic Apr 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"retries" are not used in provider.go but are used pretty much in every 1and1 resource.

"port_from": {
Type: schema.TypeInt,
Optional: true,
ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a new func that you can use as part of the helpers package:

validation.IntBetween(1, 65535),

it lives in the github.com/hashicorp/terraform/helper/validation package

return err
}

config.API.WaitForState(fw, "ACTIVE", 10, config.Retries)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are waiting for state twice

}

if raw, ok := d.GetOk("persistence"); ok {
req.Persistence = oneandone.Bool2Pointer(raw.(bool))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love the wrappers :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I remove it?

func resourceOneandOneLoadbalancerUpdate(d *schema.ResourceData, meta interface{}) error {
config := meta.(*Config)

if d.HasChange("name") || d.HasChange("description") || d.HasChange("method") || d.HasChange("persistence") || d.HasChange("persistence_time") || d.HasChange("health_check_test") || d.HasChange("healt_check_interval") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo on health_check_interval

oldR, newR := d.GetChange("rules")
oldValues := oldR.([]interface{})
newValues := newR.([]interface{})
if len(oldValues) > len(newValues) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again questioning if this is valid logic...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added acceptance test for the scenario

"time"
)

func TestAccOneandoneLoadbalancer_Basic(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would love to see a test for Import

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import is not implemented

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed ImportStatePassthrough

Type: schema.TypeString,
Optional: true,
},
"method": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to check the valid values or the casing of this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added validation for two known methods

@jasmingacic
Copy link
Author

Hi @stack72 ,

I've addressed all remarks as requested

@stack72
Copy link
Contributor

stack72 commented Apr 20, 2017

This test: TestAccOneandonePrivateNetwork_Basic doesn't finish for me - has been running for like 30 mins

Also, the website doesn't have the link to the provider docs

@jasmingacic
Copy link
Author

Here are the docs https://cloudpanel-api.1and1.com/documentation/v1/en/documentation.html
As for the test I'll get back to you

@jasmingacic
Copy link
Author

@stack72 I was able to speed it up a little by using different server image

==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/04/20 21:45:01 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/oneandone/ -v -run=TestAccOneandonePrivateNetwork_Basic -timeout 120m
=== RUN   TestAccOneandonePrivateNetwork_Basic
--- PASS: TestAccOneandonePrivateNetwork_Basic (898.95s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/oneandone      898.961s

I'm going to push the changes

@jasmingacic
Copy link
Author

==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/04/20 23:11:15 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/oneandone/ -v  -timeout 120m
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestAccOneandoneFirewall_Basic
--- PASS: TestAccOneandoneFirewall_Basic (58.00s)
=== RUN   TestAccOneandoneLoadbalancer_Basic
--- PASS: TestAccOneandoneLoadbalancer_Basic (140.70s)
=== RUN   TestAccOneandoneMonitoringPolicy_Basic
--- PASS: TestAccOneandoneMonitoringPolicy_Basic (28.18s)
=== RUN   TestAccOneandonePrivateNetwork_Basic
--- PASS: TestAccOneandonePrivateNetwork_Basic (868.68s)
=== RUN   TestAccOneandonePublicIp_Basic
--- PASS: TestAccOneandonePublicIp_Basic (43.53s)
=== RUN   TestAccOneandoneServer_Basic
--- PASS: TestAccOneandoneServer_Basic (233.72s)
=== RUN   TestAccOneandoneVpn_Basic
--- PASS: TestAccOneandoneVpn_Basic (110.39s)
=== RUN   TestAccOneandoneSharedStorage_Basic
--- PASS: TestAccOneandoneSharedStorage_Basic (60.77s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/oneandone      1543.995s```

@stack72
Copy link
Contributor

stack72 commented Apr 21, 2017

One test issue that @jasminSPC is working on but this is good to go! I am going to merge and he is going to follow up with a test fix

Paul

@stack72 stack72 merged commit 61499cf into hashicorp:master Apr 21, 2017
@jasmingacic jasmingacic deleted the oneandone branch August 2, 2017 21:02
@ghost
Copy link

ghost commented Apr 8, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants