Skip to content

Commit

Permalink
feat(update-app-base): add support to update base for application c…
Browse files Browse the repository at this point in the history
…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.

We add the model_type computed field on the application schema, and we
use it to make a decision in the planmodifier RequiresReplaceIf.

re #635
  • Loading branch information
SimoneDutto committed Jan 23, 2025
1 parent fbd1ccc commit c1e9afc
Show file tree
Hide file tree
Showing 5 changed files with 160 additions and 20 deletions.
3 changes: 2 additions & 1 deletion docs/resources/application.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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.
Expand Down
46 changes: 33 additions & 13 deletions internal/juju/applications.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,8 @@ type transformedCreateApplicationInput struct {
}

type CreateApplicationResponse struct {
AppName string
AppName string
ModelType string
}

type ReadApplicationInput struct {
Expand All @@ -284,6 +285,7 @@ type ReadApplicationResponse struct {
Channel string
Revision int
Base string
ModelType string
Series string
Units int
Trust bool
Expand All @@ -307,9 +309,9 @@ type UpdateApplicationInput struct {
Trust *bool
Expose map[string]interface{}
// Unexpose indicates what endpoints to unexpose
Unexpose []string
Config map[string]string
//Series string // Unsupported today
Unexpose []string
Config map[string]string
Base string
Placement map[string]interface{}
Constraints *constraints.Value
EndpointBindings map[string]string
Expand Down Expand Up @@ -368,9 +370,16 @@ func (c applicationsClient) CreateApplication(ctx context.Context, input *Create
// If we have managed to deploy something, now we have
// to check if we have to expose something
err = c.processExpose(applicationAPIClient, transformedInput.applicationName, transformedInput.expose)

if err != nil {
return nil, err
}
modelType, err := c.ModelType(input.ModelName)
if err != nil {
return nil, err
}
return &CreateApplicationResponse{
AppName: transformedInput.applicationName,
AppName: transformedInput.applicationName,
ModelType: modelType.String(),
}, err
}

Expand Down Expand Up @@ -1114,6 +1123,7 @@ func (c applicationsClient) ReadApplication(input *ReadApplicationInput) (*ReadA
Channel: appInfo.Channel,
Revision: charmURL.Revision,
Base: fmt.Sprintf("%s@%s", appInfo.Base.Name, baseChannel.Track),
ModelType: modelType.String(),
Series: seriesString,
Units: unitCount,
Trust: trustValue,
Expand Down Expand Up @@ -1194,7 +1204,7 @@ func (c applicationsClient) UpdateApplication(input *UpdateApplicationInput) err
// before the operations with config. Because the config params
// can be changed from one revision to another. So "Revision-Config"
// ordering will help to prevent issues with the configuration parsing.
if input.Revision != nil || input.Channel != "" || len(input.Resources) != 0 {
if input.Revision != nil || input.Channel != "" || len(input.Resources) != 0 || input.Base != "" {
setCharmConfig, err := c.computeSetCharmConfig(input, applicationAPIClient, charmsAPIClient, resourcesAPIClient)
if err != nil {
return err
Expand Down Expand Up @@ -1379,22 +1389,24 @@ func (c applicationsClient) computeSetCharmConfig(
newOrigin.Branch = strPtr(parsedChannel.Branch)
}
}
if input.Base != "" {
base, err := corebase.ParseBaseFromString(input.Base)
if err != nil {
return nil, err
}
newOrigin.Base = base
}

resolvedURL, resolvedOrigin, supportedBases, err := resolveCharm(charmsAPIClient, newURL, newOrigin)
if err != nil {
return nil, err
}

// Ensure that the new charm supports the architecture and
// operating system currently used by the deployed application.
// Ensure that the new charm supports the architecture used by the deployed application.
if oldOrigin.Architecture != resolvedOrigin.Architecture {
msg := fmt.Sprintf("the new charm does not support the current architecture %q", oldOrigin.Architecture)
return nil, errors.New(msg)
}
if !basesContain(oldOrigin.Base, supportedBases) {
msg := fmt.Sprintf("the new charm does not support the current operating system %q", oldOrigin.Base.String())
return nil, errors.New(msg)
}

// Ensure the new revision or channel is contained
// in the origin to be saved by juju when AddCharm
Expand All @@ -1406,6 +1418,14 @@ func (c applicationsClient) computeSetCharmConfig(
oldOrigin.Risk = newOrigin.Risk
oldOrigin.Branch = newOrigin.Branch
}
if input.Base != "" {
oldOrigin.Base = newOrigin.Base
}

if !basesContain(oldOrigin.Base, supportedBases) {
msg := fmt.Sprintf("the new charm does not support the current operating system %q", oldOrigin.Base.String())
return nil, errors.New(msg)
}

resultOrigin, err := charmsAPIClient.AddCharm(resolvedURL, oldOrigin, false)
if err != nil {
Expand Down
32 changes: 32 additions & 0 deletions internal/provider/modifer_applicationbaseupdate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright 2025 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

package provider

import (
"context"

"github.com/hashicorp/terraform-plugin-framework/path"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/types"
"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 modelType types.String
diags := req.State.GetAttribute(ctx, path.Root("model_type"), &modelType)
if diags.HasError() {
resp.Diagnostics.Append(diags...)
return
}
resp.RequiresReplace = modelType.ValueString() == model.IAAS.String()
}
}
33 changes: 27 additions & 6 deletions internal/provider/resource_application.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -581,7 +591,6 @@ func (r *applicationResource) Create(ctx context.Context, req resource.CreateReq
return
}
r.trace(fmt.Sprintf("read application resource %q", createResp.AppName))

// Save plan into Terraform state

// Constraints do not apply to subordinate applications. If the application
Expand All @@ -590,6 +599,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(readResp.ModelType)
planCharm.Revision = types.Int64Value(int64(readResp.Revision))
planCharm.Base = types.StringValue(readResp.Base)
planCharm.Series = types.StringValue(readResp.Series)
Expand Down Expand Up @@ -692,6 +702,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)

Expand All @@ -700,6 +716,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
Expand Down Expand Up @@ -919,16 +936,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.")
}
}

Expand Down Expand Up @@ -1051,7 +1070,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,
Expand Down Expand Up @@ -1090,6 +1110,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))
Expand Down
66 changes: 66 additions & 0 deletions internal/provider/resource_application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit c1e9afc

Please sign in to comment.