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

feat(update-app-base): add support to update base for application charms #652

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
SimoneDutto marked this conversation as resolved.
Show resolved Hide resolved
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{
hmlanigan marked this conversation as resolved.
Show resolved Hide resolved
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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: While technically harmless, this value should really be included in the CreateApplicationResponse and ReadApplicationResponse structures, rather than making a second call in each place. We're trying to keep a cleaner distinction between what the provider has to know and juju or other details.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i've add to both structs ModelType

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
Loading