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.

re juju#635
  • Loading branch information
SimoneDutto committed Jan 7, 2025
1 parent fbd1ccc commit 19c361e
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 5 deletions.
1 change: 1 addition & 0 deletions docs/resources/application.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ resource "juju_application" "this" {
- `constraints` (String) Constraints imposed on this application. Changing this value will cause the application to be destroyed and recreated by terraform.
- `endpoint_bindings` (Attributes Set) Configure endpoint bindings (see [below for nested schema](#nestedatt--endpoint_bindings))
- `expose` (Block List) Makes an application publicly available over the network (see [below for nested schema](#nestedblock--expose))
- `model_type` (String)
- `name` (String) A custom name for the application deployment. If empty, uses the charm's name.Changing this value will cause the application to be destroyed and recreated by terraform.
- `placement` (String) Specify the target location for the application's units. Changing this value will cause the application to be destroyed and recreated by terraform.
- `resources` (Map of String) Charm resources. Must evaluate to a string. A resource could be a resource revision number from CharmHub or a custom OCI image resource.
Expand Down
11 changes: 10 additions & 1 deletion internal/juju/applications.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ type UpdateApplicationInput struct {
Unexpose []string
Config map[string]string
//Series string // Unsupported today
Base string
Placement map[string]interface{}
Constraints *constraints.Value
EndpointBindings map[string]string
Expand Down Expand Up @@ -1194,7 +1195,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 @@ -1378,6 +1379,12 @@ func (c applicationsClient) computeSetCharmConfig(
if parsedChannel.Branch != "" {
newOrigin.Branch = strPtr(parsedChannel.Branch)
}
} else 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)
Expand Down Expand Up @@ -1405,6 +1412,8 @@ func (c applicationsClient) computeSetCharmConfig(
oldOrigin.Track = newOrigin.Track
oldOrigin.Risk = newOrigin.Risk
oldOrigin.Branch = newOrigin.Branch
} else if input.Base != "" {
oldOrigin.Base = newOrigin.Base
}

resultOrigin, err := charmsAPIClient.AddCharm(resolvedURL, oldOrigin, false)
Expand Down
47 changes: 43 additions & 4 deletions internal/provider/resource_application.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/juju/errors"
"github.com/juju/juju/core/constraints"
"github.com/juju/juju/core/model"
jujustorage "github.com/juju/juju/storage"

"github.com/juju/terraform-provider-juju/internal/juju"
Expand Down Expand Up @@ -82,6 +83,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 +149,14 @@ func (r *applicationResource) Schema(_ context.Context, _ resource.SchemaRequest
stringplanmodifier.RequiresReplaceIfConfigured(),
},
},
"model_type": schema.StringAttribute{
Description: "",
Computed: true,
Optional: 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 @@ -321,6 +331,18 @@ func (r *applicationResource) Schema(_ context.Context, _ resource.SchemaRequest
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
stringplanmodifier.RequiresReplaceIf(func(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()
}
}, "", ""),
},
Validators: []validator.String{
stringvalidator.ConflictsWith(path.Expressions{
Expand Down Expand Up @@ -582,6 +604,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
Expand All @@ -590,6 +617,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)
Expand Down Expand Up @@ -692,6 +720,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 +734,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 +954,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 +1088,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 +1128,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 22
Config: testAccApplicationUpdateBaseCharm(modelName, "[email protected]"),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("juju_application.this", "charm.0.base", "[email protected]"),
),
},
{
// move back to latest/stable
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 19c361e

Please sign in to comment.