-
Notifications
You must be signed in to change notification settings - Fork 45
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(update-app-base): add support to update
base
for application c…
…harms This PR adds support to update the base in application charms by requiring a replace in case of a machine charm, and perform the upgrade in case of a k8s charm. re #635
- Loading branch information
1 parent
fbd1ccc
commit 5431e7c
Showing
5 changed files
with
141 additions
and
7 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -74,6 +74,7 @@ Notes: | |
### Read-Only | ||
|
||
- `id` (String) The ID of this resource. | ||
- `model_type` (String) The type of the model where the application is to be deployed. It is a computed field and is needed to determine if the application should be replaced or updated in case of base updates. | ||
- `principal` (Boolean, Deprecated) Whether this is a Principal application | ||
|
||
<a id="nestedblock--charm"></a> | ||
|
@@ -85,7 +86,7 @@ Required: | |
|
||
Optional: | ||
|
||
- `base` (String) The operating system on which to deploy. E.g. [email protected]. | ||
- `base` (String) The operating system on which to deploy. E.g. [email protected]. Changing this value for machine charms will trigger a replace by terraform. | ||
- `channel` (String) The channel to use when deploying a charm. Specified as \<track>/\<risk>/\<branch>. | ||
- `revision` (Number) The revision of the charm to deploy. During the update phase, the charm revision should be update before config update, to avoid issues with config parameters parsing. | ||
- `series` (String, Deprecated) The series on which to deploy. | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
// Copyright 2024 Canonical Ltd. | ||
// Licensed under the AGPLv3, see LICENCE file for details. | ||
|
||
package provider | ||
|
||
import ( | ||
"context" | ||
|
||
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" | ||
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" | ||
"github.com/juju/juju/core/model" | ||
) | ||
|
||
// baseApplicationRequiresReplaceIf is a plan modifier that sets the RequiresReplace field if the | ||
// model type is IAAS. The reason is that with CAAS the application can be updated in place. | ||
// With IAAS the application needs to be replaced. To make this decision the model type is needed. | ||
// Since you can't access the juju client in the plan modifiers we've added a computed field `model_type`. | ||
// This is set in the state by means of the `stringplanmodifier.UseStateForUnknown()`, so when we update the base | ||
// is always guaranteed to be set. | ||
func baseApplicationRequiresReplaceIf(ctx context.Context, req planmodifier.StringRequest, resp *stringplanmodifier.RequiresReplaceIfFuncResponse) { | ||
if req.State.Raw.IsKnown() { | ||
var state applicationResourceModel | ||
diags := req.State.Get(ctx, &state) | ||
if diags.HasError() { | ||
resp.Diagnostics.Append(diags...) | ||
return | ||
} | ||
modelType := state.ModelType.ValueString() | ||
resp.RequiresReplace = modelType == model.IAAS.String() | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -82,6 +82,7 @@ type applicationResourceModel struct { | |
Constraints types.String `tfsdk:"constraints"` | ||
Expose types.List `tfsdk:"expose"` | ||
ModelName types.String `tfsdk:"model"` | ||
ModelType types.String `tfsdk:"model_type"` | ||
Placement types.String `tfsdk:"placement"` | ||
EndpointBindings types.Set `tfsdk:"endpoint_bindings"` | ||
Resources types.Map `tfsdk:"resources"` | ||
|
@@ -147,6 +148,14 @@ func (r *applicationResource) Schema(_ context.Context, _ resource.SchemaRequest | |
stringplanmodifier.RequiresReplaceIfConfigured(), | ||
}, | ||
}, | ||
"model_type": schema.StringAttribute{ | ||
Description: "The type of the model where the application is to be deployed. It is a computed field and " + | ||
"is needed to determine if the application should be replaced or updated in case of base updates.", | ||
Computed: true, | ||
PlanModifiers: []planmodifier.String{ | ||
stringplanmodifier.UseStateForUnknown(), | ||
}, | ||
}, | ||
"units": schema.Int64Attribute{ | ||
Description: "The number of application units to deploy for the charm.", | ||
Optional: true, | ||
|
@@ -316,11 +325,12 @@ func (r *applicationResource) Schema(_ context.Context, _ resource.SchemaRequest | |
DeprecationMessage: "Configure base instead. This attribute will be removed in the next major version of the provider.", | ||
}, | ||
BaseKey: schema.StringAttribute{ | ||
Description: "The operating system on which to deploy. E.g. [email protected].", | ||
Description: "The operating system on which to deploy. E.g. [email protected]. Changing this value for machine charms will trigger a replace by terraform.", | ||
Optional: true, | ||
Computed: true, | ||
PlanModifiers: []planmodifier.String{ | ||
stringplanmodifier.UseStateForUnknown(), | ||
stringplanmodifier.RequiresReplaceIf(baseApplicationRequiresReplaceIf, "", ""), | ||
}, | ||
Validators: []validator.String{ | ||
stringvalidator.ConflictsWith(path.Expressions{ | ||
|
@@ -582,6 +592,11 @@ func (r *applicationResource) Create(ctx context.Context, req resource.CreateReq | |
} | ||
r.trace(fmt.Sprintf("read application resource %q", createResp.AppName)) | ||
|
||
modelType, err := r.client.Applications.ModelType(modelName) | ||
if err != nil { | ||
resp.Diagnostics.Append(handleApplicationNotFoundError(ctx, err, &resp.State)...) | ||
return | ||
} | ||
// Save plan into Terraform state | ||
|
||
// Constraints do not apply to subordinate applications. If the application | ||
|
@@ -590,6 +605,7 @@ func (r *applicationResource) Create(ctx context.Context, req resource.CreateReq | |
plan.Placement = types.StringValue(readResp.Placement) | ||
plan.Principal = types.BoolNull() | ||
plan.ApplicationName = types.StringValue(createResp.AppName) | ||
plan.ModelType = types.StringValue(modelType.String()) | ||
planCharm.Revision = types.Int64Value(int64(readResp.Revision)) | ||
planCharm.Base = types.StringValue(readResp.Base) | ||
planCharm.Series = types.StringValue(readResp.Series) | ||
|
@@ -692,6 +708,12 @@ func (r *applicationResource) Read(ctx context.Context, req resource.ReadRequest | |
} | ||
r.trace("read application", map[string]interface{}{"resource": appName, "response": response}) | ||
|
||
modelType, err := r.client.Applications.ModelType(modelName) | ||
if err != nil { | ||
resp.Diagnostics.Append(handleApplicationNotFoundError(ctx, err, &resp.State)...) | ||
return | ||
} | ||
|
||
state.ApplicationName = types.StringValue(appName) | ||
state.ModelName = types.StringValue(modelName) | ||
|
||
|
@@ -700,6 +722,7 @@ func (r *applicationResource) Read(ctx context.Context, req resource.ReadRequest | |
state.Placement = types.StringValue(response.Placement) | ||
state.Principal = types.BoolNull() | ||
state.UnitCount = types.Int64Value(int64(response.Units)) | ||
state.ModelType = types.StringValue(modelType.String()) | ||
state.Trust = types.BoolValue(response.Trust) | ||
|
||
// state requiring transformation | ||
|
@@ -919,16 +942,18 @@ func (r *applicationResource) Update(ctx context.Context, req resource.UpdateReq | |
} else if !planCharm.Revision.Equal(stateCharm.Revision) { | ||
updateApplicationInput.Revision = intPtr(planCharm.Revision) | ||
} | ||
|
||
if !planCharm.Series.Equal(stateCharm.Series) || !planCharm.Base.Equal(stateCharm.Base) { | ||
if !planCharm.Base.Equal(stateCharm.Base) { | ||
updateApplicationInput.Base = planCharm.Base.ValueString() | ||
} | ||
if !planCharm.Series.Equal(stateCharm.Series) { | ||
// This violates Terraform's declarative model. We could implement | ||
// `juju set-application-base`, usually used after `upgrade-machine`, | ||
// which would change the operating system used for future units of | ||
// the application provided the charm supported it, but not change | ||
// the current. This provider does not implement an equivalent to | ||
// `upgrade-machine`. There is also a question of how to handle a | ||
// change to series, revision and channel at the same time. | ||
resp.Diagnostics.AddWarning("Not Supported", "Changing an application's operating system after deploy.") | ||
resp.Diagnostics.AddWarning("Not Supported", "Changing operating system's series after deploy.") | ||
} | ||
} | ||
|
||
|
@@ -1051,7 +1076,8 @@ func (r *applicationResource) Update(ctx context.Context, req resource.UpdateReq | |
if updateApplicationInput.Channel != "" || | ||
updateApplicationInput.Revision != nil || | ||
updateApplicationInput.Placement != nil || | ||
updateApplicationInput.Units != nil { | ||
updateApplicationInput.Units != nil || | ||
updateApplicationInput.Base != "" { | ||
readResp, err := r.client.Applications.ReadApplicationWithRetryOnNotFound(ctx, &juju.ReadApplicationInput{ | ||
ModelName: updateApplicationInput.ModelName, | ||
AppName: updateApplicationInput.AppName, | ||
|
@@ -1090,6 +1116,7 @@ func (r *applicationResource) Update(ctx context.Context, req resource.UpdateReq | |
} | ||
} | ||
|
||
plan.ModelType = state.ModelType | ||
plan.ID = types.StringValue(newAppID(plan.ModelName.ValueString(), plan.ApplicationName.ValueString())) | ||
plan.Principal = types.BoolNull() | ||
r.trace("Updated", applicationResourceModelForLogging(ctx, &plan)) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -276,6 +276,37 @@ func TestAcc_CharmUpdates(t *testing.T) { | |
}) | ||
} | ||
|
||
func TestAcc_CharmUpdateBase(t *testing.T) { | ||
modelName := acctest.RandomWithPrefix("tf-test-charmbaseupdates") | ||
|
||
resource.ParallelTest(t, resource.TestCase{ | ||
PreCheck: func() { testAccPreCheck(t) }, | ||
ProtoV6ProviderFactories: frameworkProviderFactories, | ||
Steps: []resource.TestStep{ | ||
{ | ||
Config: testAccApplicationUpdateBaseCharm(modelName, "[email protected]"), | ||
Check: resource.ComposeTestCheckFunc( | ||
resource.TestCheckResourceAttr("juju_application.this", "charm.0.base", "[email protected]"), | ||
), | ||
}, | ||
{ | ||
// move to base ubuntu 20.04 | ||
Config: testAccApplicationUpdateBaseCharm(modelName, "[email protected]"), | ||
Check: resource.ComposeTestCheckFunc( | ||
resource.TestCheckResourceAttr("juju_application.this", "charm.0.base", "[email protected]"), | ||
), | ||
}, | ||
{ | ||
// move back to ubuntu 22.04 | ||
Config: testAccApplicationUpdateBaseCharm(modelName, "[email protected]"), | ||
Check: resource.ComposeTestCheckFunc( | ||
resource.TestCheckResourceAttr("juju_application.this", "charm.0.base", "[email protected]"), | ||
), | ||
}, | ||
}, | ||
}) | ||
} | ||
|
||
func TestAcc_ResourceRevisionUpdatesLXD(t *testing.T) { | ||
if testingCloud != LXDCloudTesting { | ||
t.Skip(t.Name() + " only runs with LXD") | ||
|
@@ -1021,6 +1052,41 @@ func testAccResourceApplicationUpdatesCharm(modelName string, channel string) st | |
} | ||
} | ||
|
||
func testAccApplicationUpdateBaseCharm(modelName string, base string) string { | ||
if testingCloud == LXDCloudTesting { | ||
return fmt.Sprintf(` | ||
resource "juju_model" "this" { | ||
name = %q | ||
} | ||
resource "juju_application" "this" { | ||
model = juju_model.this.name | ||
name = "test-app" | ||
charm { | ||
name = "ubuntu" | ||
base = %q | ||
} | ||
} | ||
`, modelName, base) | ||
} else { | ||
return fmt.Sprintf(` | ||
resource "juju_model" "this" { | ||
name = %q | ||
} | ||
resource "juju_application" "this" { | ||
model = juju_model.this.name | ||
name = "test-app" | ||
charm { | ||
name = "coredns" | ||
channel = "1.25/stable" | ||
base = %q | ||
} | ||
} | ||
`, modelName, base) | ||
} | ||
} | ||
|
||
// testAccResourceApplicationConstraints will return two set for constraint | ||
// applications. The version to be used in K8s sets the juju-external-hostname | ||
// because we set the expose parameter. | ||
|