STACKITRCO-187 - Add option iaas API param agent#1113
STACKITRCO-187 - Add option iaas API param agent#1113aeter wants to merge 1 commit intostackitcloud:mainfrom
Conversation
44c1522 to
699af50
Compare
|
This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it. |
cgoetz-inovex
left a comment
There was a problem hiding this comment.
lgtm besides the docs
d208dba to
91a621b
Compare
91a621b to
7c34d59
Compare
Adds a terraform `stackit_server` option for the iaas ( _create server_ ) API param:
`"agent": {"provisioned": true}`
ref STACKITRCO-187
ref: stackitcloud/stackit-cli#1213
review url: stackitcloud#1113
---
Tests:
* ran `make fmt`, `make generate-docs`, `make lint`
* ran unit tests
```
[~/terraform-provider-stackit] go test stackit/internal/services/iaas/server/*
ok command-line-arguments 15.005s
[~/terraform-provider-stackit] make test
...
ok github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/iaas/server 15.006s coverage: 33.0% of statements
...
[~/terraform-provider-stackit]
```
* Tested: with a locally-configured terraform, by adding, changing, deleting `agent`-related parts of a tf config.
Signed-off-by: Adrian Nackov <adrian.nackov@digits.schwarz>
|
please review. I've updated the code as requested - added acceptance testsA server created without sending 'provisioning' flag via the CLI - and then imported into terraform:A server created in terraform with defaults (no 'agent' block):A server created in terraform like this. Note the ALWAYS->INHERIT in datasource, which can/should be fixed when the API is fixed one day: |
Adds a terraform `stackit_server` option for the iaas ( _create server_ ) API param:
`"agent": {"provisioned": true}`
ref STACKITRCO-187
ref: stackitcloud/stackit-cli#1213
review url: stackitcloud#1113
---
Tests:
* ran `make fmt`, `make generate-docs`, `make lint`
* ran unit tests
```
[~/terraform-provider-stackit] go test stackit/internal/services/iaas/server/*
ok command-line-arguments 15.005s
[~/terraform-provider-stackit] make test
...
ok github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/iaas/server 15.006s coverage: 33.0% of statements
...
[~/terraform-provider-stackit]
```
* Tested: with a locally-configured terraform, by adding, changing, deleting `agent`-related parts of a tf config.
Signed-off-by: Adrian Nackov <adrian.nackov@digits.schwarz>
7c34d59 to
8942b1e
Compare
Adds a terraform `stackit_server` option for the iaas ( _create server_ ) API param:
`"agent": {"provisioned": true}`
ref STACKITRCO-187
ref: stackitcloud/stackit-cli#1213
review url: stackitcloud#1113
---
Tests:
* ran `make fmt`, `make generate-docs`, `make lint`
* ran unit tests
```
[~/terraform-provider-stackit] go test stackit/internal/services/iaas/server/*
ok command-line-arguments 15.005s
[~/terraform-provider-stackit] make test
...
ok github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/iaas/server 15.006s coverage: 33.0% of statements
...
[~/terraform-provider-stackit]
```
* Tested: with a locally-configured terraform, by adding, changing, deleting `agent`-related parts of a tf config.
Signed-off-by: Adrian Nackov <adrian.nackov@digits.schwarz>
8942b1e to
be8d15a
Compare
|
updated per review |
| resource.TestCheckResourceAttrSet("stackit_server.server", "launched_at"), | ||
| resource.TestCheckResourceAttrSet("stackit_server.server", "updated_at"), | ||
| resource.TestCheckResourceAttr("stackit_server.server", "agent.provisioning_policy", "INHERIT"), | ||
| resource.TestCheckNoResourceAttr("stackit_server.server", "agent.provisioned"), |
There was a problem hiding this comment.
Shouldn't this be either true or false always?
What does a nil response for the provisioned field from the API mean?
There was a problem hiding this comment.
The default is "INHERIT", it's neither true nor false. The default is not false.
A nil response from API (i.e. a missing 'agent' key) means using local tfstate and defaulting to INHERIT.
There was a problem hiding this comment.
Sorry, my question was independent from Terraform - what does it mean technically if the API response is missing the provisioned.provisioned field? Does it mean an agent was provisioned or not?
So in fact, what's the difference between a server for which in the API response the provisioned.provisioned is null/missing vs. a server for which the API response has provisioned.provisioned set to false?
There was a problem hiding this comment.
I think the current IAAS API gives 2 responses:
-
like:
{"id": "...", "network": "...", "agent": {"provisioned": true}}
Fortrue, the agent is sure to be provisioned on the server. Forfalse- it's sure the agent is not provisioned on the server.
This API response is seen only when the server was explicitly created (POST /servers) by specifying{"agent": {"provisioned": true|false}}next to the other keys in the JSON payload. -
like
{"id": "...", "network": "..."}- noagentkey
The agent may be provisioned. Depends on the default for the server image (true|false). Also, I think, this image default could be changeable with enough rights in the API.
This API response is seen when the server was created (POST /servers) without mentioning anagentkey in the JSON payload.
There was a problem hiding this comment.
So for the 2nd case you mentioned the question to "Is there an agent provisioned on the server?" is "Schrödingers cat"? 😲
There was a problem hiding this comment.
Yes, I think so. Thus the 3 cases (inherit, always, never) at the terraform provider's PR code. The /images api cannot be used to determine the provisioning state reliably (as the image default value may be changed). I think the /command Run Commands apis currently don't provide an endpoint for the agent provisioning state either. In the future, it's likely there will be api improvements.
| }, | ||
| }, | ||
| "provisioning_policy": schema.StringAttribute{ | ||
| Description: "Agent provisioning policy: \"ALWAYS\", \"NEVER\", or \"INHERIT\". \"INHERIT\" follows the image default value.", |
There was a problem hiding this comment.
| Description: "Agent provisioning policy: \"ALWAYS\", \"NEVER\", or \"INHERIT\". \"INHERIT\" follows the image default value.", | |
| Description: "Agent provisioning policy: `ALWAYS`, `NEVER`, or `INHERIT`. `INHERIT` follows the image default value.", |
You can use markdown formatting here 😅
There was a problem hiding this comment.
I want quotes in the docs. Like:
"INHERIT"
and not like
INHERIT
There was a problem hiding this comment.
And I would like you to stick to our conventions 😅
You could even use these util func to format your values if you want:
terraform-provider-stackit/stackit/internal/utils/utils.go
Lines 141 to 152 in 7fb8cdb
There was a problem hiding this comment.
Please don't resolve such conversations silently if you want to discuss about the comments instead of resolving them in your code. Makes review pretty hard if I have to check every resolved comment over and over again. Thanks 😊
Adds a terraform `stackit_server` option for the iaas ( _create server_ ) API param:
`"agent": {"provisioned": true}`
ref STACKITRCO-187
ref: stackitcloud/stackit-cli#1213
review url: stackitcloud#1113
---
Tests:
* ran `make fmt`, `make generate-docs`, `make lint`
* ran unit tests
```
[~/terraform-provider-stackit] go test stackit/internal/services/iaas/server/*
ok command-line-arguments 15.005s
[~/terraform-provider-stackit] make test
...
ok github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/iaas/server 15.006s coverage: 33.0% of statements
...
[~/terraform-provider-stackit]
```
* Tested: with a locally-configured terraform, by adding, changing, deleting `agent`-related parts of a tf config.
Signed-off-by: Adrian Nackov <adrian.nackov@digits.schwarz>
be8d15a to
b7e25e0
Compare
Adds a terraform
stackit_serveroption for the iaas ( create server ) API param:"agent": {"provisioned": true}ref STACKITRCO-187
ref: stackitcloud/stackit-cli#1213
Tests:
* ran
make fmt,make generate-docs,make lintDescription
relates to STACKITRCO-187
Checklist
make fmtexamples/directory)make generate-docs(will be checked by CI)make test(will be checked by CI)make lint(will be checked by CI)