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

Change gridscale_server resource hardware_profile default for new instances #141

Open
bkircher opened this issue Mar 17, 2021 · 12 comments
Open

Comments

@bkircher
Copy link
Contributor

gridscale_server resource still produces a "default" hardware profile instance. I believe we should switch here to "q35".

Expected Behavior

resource "gridscale_server" "foo" {
  name   = "foo"
  cores  = 1
  memory = 2
  power  = true
}

This should produce an instance with Q35 profile.

Basically this should behave as I would have given q35 explicitly

  #
  hardware_profile = "q35"
  #

Actual Behavior

It produces an instance with "Default" profile.

Compatibility

Note, we only want to change the default for new resources here. A TF provider update should not change existing instances.

I don't really know if we should categorize this as a breaking change. If users still rely on having pc-i440fx-2.4 if they don't specify a profile this would break their flow. Maybe we can get away with a major version bump and a notice in the release notes?

@nvthongswansea
Copy link
Contributor

@bkircher it relates to this gsclient-go issue 161, which should be solved in the API level.

@bkircher
Copy link
Contributor Author

No, I think API is already done here. I guess there's schema default?

Ah, yes, got it here https://github.com/gridscale/terraform-provider-gridscale/blob/master/gridscale/resource_gridscale_server.go#L61

@bkircher
Copy link
Contributor Author

@itakouna Could you check if this would break compatibility for you guys in GSK provisioning if we change the default here?

@nvthongswansea
Copy link
Contributor

No, I think API is already done here. I guess there's schema default?

Ah, yes, got it here https://github.com/gridscale/terraform-provider-gridscale/blob/master/gridscale/resource_gridscale_server.go#L61

I've just asked @MarcHarriss, he's confirmed that when you set hardware_profile to "Default" (via API, or panel), the q35 hardware profile is used under the hood.

@bkircher
Copy link
Contributor Author

Erm, not 100% accurate.

The profile named "default" is not the default anymore. I know, sounds confusing. It is confusing 😄 and will be changed by API and front-end guys eventually.

The default now in panel and APIs is "q35". Which is correct. But we in TF provider explicitly set "default" as default here in this schema; which is wrong. We should make it either "q35" as well or just leave it unspecified to use the default dictated by the API (which is "q35").

(Formulating this sentence was fun 😄 )

I guess, this is why having a thing with the name "default" is almost always a bad idea 🙂

@MarcHarriss
Copy link
Member

What I meant was - when you don't specify a hardware profile, the Q35 is used a the "default" -> exactly as Ben describes here @nvthongswansea

@itakouna itakouna reopened this Mar 19, 2021
@itakouna
Copy link
Contributor

@bkircher we should add hardware_profile = "default" to TF files for keeping backward compatibility.

@itakouna
Copy link
Contributor

@bkircher I strongly recommend bumping the major version of gs TF, when solving the issue.

@bkircher
Copy link
Contributor Author

OK, thanks. So this will be a breaking change and needs special attention when releasing.

Do we do this in a 1.9.0 or in a 2.0.0? Any thoughts? @itakouna @nvthongswansea

Strictly semantic versioning would make this 2.0.0 I think, but that would mean we deliver this around May because we have other breaking changes in that package.

@itakouna
Copy link
Contributor

Even though we release a major version this still might impact customers who do not pin GS TF version. In this case, all servers will be recreated.

terraform {
  required_providers {
    gridscale = "~> 1.7.4"
}

@nvthongswansea
Copy link
Contributor

@bkircher @itakouna please be patient. I am trying some means to make it back-compat. I will post any updates here.

@nvthongswansea
Copy link
Contributor

@bkircher @itakouna Regarding to #144, the default hardware profile of a server is now determined by the gs backend (which is q35 at the moment of writing) when users don't set hardware_profile. I also tested the back compatibility on my machine, and it works (for me).

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

No branches or pull requests

4 participants