Skip to content
This repository has been archived by the owner on Nov 14, 2022. It is now read-only.

port_mapping.name, port_definition.[name|lables], health_check.port a… #65

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

cihangirbesiktas
Copy link
Contributor

…nd volume.external are added

@cihangirbesiktas
Copy link
Contributor Author

cihangirbesiktas commented Aug 2, 2017

I try to resolve this test case, but the test is posting a json definition with container.volumes.volume.external = {} when there is no external in the definition that causes marathon api to produce error. The code is running in our environment without an error.

@luisdavim
Copy link

The test that @cihangirbesiktas mentioned is now fixed but we have another problem:

* marathon_app.app-create-example: Marathon API error: Object is not valid (path: '/upgradeStrategy/maximumOverCapacity' errors: got 0.3, expected 0.0; path: '/container/volumes(1)' errors: Feature external_volumes is not enabled. Enable with --enable_features external_volumes); path: '/container/portMappings(1)/name' errors: must fully match regular expression '^[a-z0-9-]+$')

The instance of marathon that we are using doesn't have --enable_features external_volumes @nicgrayson any ideas on how to fix this?

@luisdavim
Copy link

Thanks @calvernaz

@calvernaz
Copy link

The failing test has wrong expectations but I'm not sure where to fix it. The expected value is wrong.

@@ -1163,6 +1237,11 @@ func mutateResourceToApplication(d *schema.ResourceData) *marathon.Application {
}
}

if prop, ok := mapStruct["port"]; ok {
prop := prop.(int)
healthCheck.Port = &prop
Copy link
Contributor

Choose a reason for hiding this comment

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

This will always set port to 0, which is wrong in case you're using portIndex.

I'd suggest to add an conditional to check if it's greater than 0.

			if prop, ok := mapStruct["port"]; ok {
				prop := prop.(int)
				if prop > 0 {
					healthCheck.Port = &prop
				}
			}

@cihangirbesiktas
Copy link
Contributor Author

--- FAIL: TestAccMarathonApp_basic (5.76s)
testing.go:268: Step 0 error: Error applying: 1 error(s) occurred:

	* marathon_app.app-create-example: 1 error(s) occurred:
	
	* marathon_app.app-create-example: Marathon API error: Object is not valid (path: '/upgradeStrategy/maximumOverCapacity' errors: got 0.3, expected 0.0)

I did not change upgradeStrategy but getting this error, it seems the test env is not set up properly, I could not find out the problem but it would be better if it is fixed.

@kamsz
Copy link
Contributor

kamsz commented Aug 22, 2017

@cihangirbesiktas I believe this happens because you've external volume attached in example.tf. In such case, upgrade strategy has to be set to 0, because Marathon cannot start an additional task (since volume can be mounted to only one task).

@cihangirbesiktas
Copy link
Contributor Author

@kamsz but when I set the maximum over capacity to 0.0, I got the following error:

--- FAIL: TestAccMarathonApp_basic (5.05s)
testing.go:268: Step 0 error: Error applying: 1 error(s) occurred:

            * marathon_app.app-create-example: 1 error(s) occurred:

            * marathon_app.app-create-example: Marathon API error: Object is not valid (path: '/upgradeStrategy/maximumOverCapacity' errors: got 1.0, expected 0.0)

@luisdavim
Copy link

maybe @nicgrayson could give us a hand here?

@cihangirbesiktas
Copy link
Contributor Author

I found out that schema.ResourceData.GetOk function returns false when the value of a key is zero, but we need the zero value in /upgradeStrategy/maximumOverCapacity. There is another function schema.ResourceData.GetOkExists which returns true regardless of zero value, but it is in the latest repo of github.com/hashicorp/terraform/helper/schema. Could @nicgrayson update the repos?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants