Skip to content

Commit

Permalink
Merge pull request #325 from hmlanigan/base-in-application
Browse files Browse the repository at this point in the history
Base in application
  • Loading branch information
hmlanigan authored Oct 20, 2023
2 parents 3548c49 + 667cf44 commit b55c592
Show file tree
Hide file tree
Showing 9 changed files with 121 additions and 73 deletions.
3 changes: 2 additions & 1 deletion docs/resources/application.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,10 @@ Required:

Optional:

- `base` (String) The operating system on which to deploy. E.g. [email protected].
- `channel` (String) The channel to use when deploying a charm. Specified as \<track>/\<risk>/\<branch>.
- `revision` (Number) The revision of the charm to deploy.
- `series` (String) The series on which to deploy.
- `series` (String, Deprecated) The series on which to deploy.


<a id="nestedblock--expose"></a>
Expand Down
2 changes: 1 addition & 1 deletion docs/resources/machine.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ resource "juju_machine" "this_machine" {

### Optional

- `base` (String) The operating system series to install on the new machine(s).
- `base` (String) The operating system to install on the new machine(s). E.g. [email protected].
- `constraints` (String) Machine constraints that overwrite those available from 'juju get-model-constraints' and provider's defaults.
- `disks` (String) Storage constraints for disks to attach to the machine(s).
- `name` (String) A name for the machine resource in Terraform.
Expand Down
85 changes: 54 additions & 31 deletions internal/juju/applications.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ type CreateApplicationInput struct {
ModelName string
CharmName string
CharmChannel string
CharmBase string
CharmSeries string
CharmRevision int
Units int
Expand All @@ -112,9 +113,7 @@ type CreateApplicationInput struct {
}

type CreateApplicationResponse struct {
AppName string
Revision int
Series string
AppName string
}

type ReadApplicationInput struct {
Expand All @@ -126,6 +125,7 @@ type ReadApplicationResponse struct {
Name string
Channel string
Revision int
Base string
Series string
Units int
Trust bool
Expand Down Expand Up @@ -213,26 +213,40 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C
return nil, fmt.Errorf("specifying a revision requires a channel for future upgrades")
}

// The architecture constraint is required to find the proper
// platform to deploy. First look at constraints provided by
// the user, fall back to any model constraints.
var platformCons constraints.Value
if input.Constraints.HasArch() {
platformCons = input.Constraints
} else {
platformCons, err = modelconfigAPIClient.GetModelConstraints()
// Look at input.CharmBase and input.CharmSeries for an operating
// system to deploy with. Only one is allowed and Charm Base is
// preferred. Keep the data as a Series for now as, the
// DeducePlatform method expects a series to be provided, not a
// base. Luckily, the DeduceOrigin method returns an origin which
// does contain the base and a series.
var userSuppliedSeries string
if input.CharmBase != "" {
b, err := series.ParseBaseFromString(input.CharmBase)
if err != nil {
return nil, err
}
userSuppliedSeries, err = series.GetSeriesFromBase(b)
if err != nil {
return nil, err
}
} else if input.CharmSeries != "" {
userSuppliedSeries = input.CharmSeries
}
platformCons, err := modelconfigAPIClient.GetModelConstraints()
if err != nil {
return nil, err
}
platform, err := utils.DeducePlatform(constraints.Value{}, input.CharmSeries, platformCons)
platform, err := utils.DeducePlatform(input.Constraints, userSuppliedSeries, platformCons)
if err != nil {
return nil, err
}

urlForOrigin := charmURL
if input.CharmRevision != UnspecifiedRevision {
urlForOrigin = urlForOrigin.WithRevision(input.CharmRevision)
}
urlForOrigin = urlForOrigin.WithSeries(userSuppliedSeries)

origin, err := utils.DeduceOrigin(urlForOrigin, channel, platform)
if err != nil {
return nil, err
Expand All @@ -249,12 +263,17 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C
return nil, jujuerrors.NotSupportedf("deploying bundles")
}

seriesToUse, err := c.seriesToUse(modelconfigAPIClient, input.CharmSeries, resolvedOrigin.Series, set.NewStrings(supportedSeries...))
seriesToUse, err := c.seriesToUse(modelconfigAPIClient, userSuppliedSeries, resolvedOrigin.Series, set.NewStrings(supportedSeries...))
if err != nil {
return nil, err
}
if input.CharmSeries != "" && seriesToUse != input.CharmSeries {
return nil, jujuerrors.Errorf("juju controller bug (LP 2039179), deploy will have operating system different from request. ")
if userSuppliedSeries != "" && seriesToUse != userSuppliedSeries {
// Ignore errors, the series have already been vetted above.
userBase, _ := series.GetBaseFromSeries(userSuppliedSeries)
suggestedBase, _ := series.GetBaseFromSeries(seriesToUse)
return nil, jujuerrors.Errorf(
"juju bug (LP 2039179), requested base %q does not match base %q found for charm.",
userBase, suggestedBase)
}

// Add the charm to the model
Expand Down Expand Up @@ -303,34 +322,28 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C
CharmID: charmID,
ApplicationName: appName,
NumUnits: input.Units,
Series: resultOrigin.Series,
CharmOrigin: resultOrigin,
Config: appConfig,
Cons: input.Constraints,
Resources: resources,
Placement: placements,
// Still supply series, to be compatible with juju 2.9 controllers.
// 3.x controllers will only use the CharmOrigin and its base.
Series: resultOrigin.Series,
CharmOrigin: resultOrigin,
Config: appConfig,
Cons: input.Constraints,
Resources: resources,
Placement: placements,
}
c.Tracef("Calling Deploy", map[string]interface{}{"args": args})
err = applicationAPIClient.Deploy(args)

if err != nil {
// unfortunate error during deployment
// TODO: 01-Aug-2023
// Why are we returning data on a failure to deploy?
return &CreateApplicationResponse{
AppName: appName,
Revision: *origin.Revision,
Series: seriesToUse,
}, err
return nil, err
}

// If we have managed to deploy something, now we have
// to check if we have to expose something
err = c.processExpose(applicationAPIClient, appName, input.Expose)
return &CreateApplicationResponse{
AppName: appName,
Revision: *origin.Revision,
Series: seriesToUse,
AppName: appName,
}, err
}

Expand Down Expand Up @@ -385,6 +398,8 @@ func (c applicationsClient) supportedWorkloadSeries(imageStream string) (set.Str
// Note, we are re-implementing the logic of series_selector in juju code as it's
// a private object.
func (c applicationsClient) seriesToUse(modelconfigAPIClient *apimodelconfig.Client, inputSeries, suggestedSeries string, charmSeries set.Strings) (string, error) {
c.Tracef("seriesToUse", map[string]interface{}{"inputSeries": inputSeries, "suggestedSeries": suggestedSeries, "charmSeries": charmSeries.Values()})

attrs, err := modelconfigAPIClient.ModelGet()
if err != nil {
return "", jujuerrors.Wrap(err, errors.New("cannot fetch model settings"))
Expand Down Expand Up @@ -705,11 +720,19 @@ func (c applicationsClient) ReadApplication(input *ReadApplicationInput) (*ReadA
exposed["spaces"] = spaces
exposed["cidrs"] = cidrs
}
// ParseChannel to send back a base without the risk.
// Having the risk will cause issues with the provider
// saving a different value than the user did.
baseChannel, err := series.ParseChannel(appInfo.Base.Channel)
if err != nil {
return nil, jujuerrors.Annotate(err, "failed parse channel for base")
}

response := &ReadApplicationResponse{
Name: charmURL.Name,
Channel: appInfo.Channel,
Revision: charmURL.Revision,
Base: fmt.Sprintf("%s@%s", appInfo.Base.Name, baseChannel.Track),
Series: appInfo.Series,
Units: unitCount,
Trust: trustValue,
Expand Down
10 changes: 5 additions & 5 deletions internal/provider/data_source_machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestAcc_DataSourceMachine_Edge(t *testing.T) {
ProtoV6ProviderFactories: frameworkProviderFactories,
Steps: []resource.TestStep{
{
Config: testAccDataSourceMachine(modelName),
Config: testAccDataSourceMachine(modelName, "base = \"[email protected]\""),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("data.juju_machine.machine", "model", modelName),
),
Expand All @@ -47,7 +47,7 @@ func TestAcc_DataSourceMachine_Stable(t *testing.T) {
},
Steps: []resource.TestStep{
{
Config: testAccDataSourceMachine(modelName),
Config: testAccDataSourceMachine(modelName, "series = \"jammy\""),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("data.juju_machine.machine", "model", modelName),
),
Expand All @@ -56,7 +56,7 @@ func TestAcc_DataSourceMachine_Stable(t *testing.T) {
})
}

func testAccDataSourceMachine(modelName string) string {
func testAccDataSourceMachine(modelName, os string) string {
return fmt.Sprintf(`
resource "juju_model" "model" {
name = %q
Expand All @@ -65,11 +65,11 @@ resource "juju_model" "model" {
resource "juju_machine" "machine" {
model = juju_model.model.name
name = "machine"
series = "jammy"
%s
}
data "juju_machine" "machine" {
model = juju_model.model.name
machine_id = juju_machine.machine.machine_id
}`, modelName)
}`, modelName, os)
}
10 changes: 5 additions & 5 deletions internal/provider/data_source_offer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func TestAcc_DataSourceOffer_Edge(t *testing.T) {
ProtoV6ProviderFactories: frameworkProviderFactories,
Steps: []resource.TestStep{
{
Config: testAccDataSourceOffer(modelName, offerName),
Config: testAccDataSourceOffer(modelName, "base = \"[email protected]\"", offerName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("data.juju_offer.this", "model", modelName),
resource.TestCheckResourceAttr("data.juju_offer.this", "name", offerName),
Expand All @@ -46,7 +46,7 @@ func TestAcc_DataSourceOffer_Stable(t *testing.T) {
},
Steps: []resource.TestStep{
{
Config: testAccDataSourceOffer(modelName, offerName),
Config: testAccDataSourceOffer(modelName, "series = \"jammy\"", offerName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("data.juju_offer.this", "model", modelName),
resource.TestCheckResourceAttr("data.juju_offer.this", "name", offerName),
Expand All @@ -56,7 +56,7 @@ func TestAcc_DataSourceOffer_Stable(t *testing.T) {
})
}

func testAccDataSourceOffer(modelName string, offerName string) string {
func testAccDataSourceOffer(modelName, os, offerName string) string {
return fmt.Sprintf(`
resource "juju_model" "this" {
name = %q
Expand All @@ -68,7 +68,7 @@ resource "juju_application" "this" {
charm {
name = "postgresql"
series = "jammy"
%s
}
}
Expand All @@ -82,5 +82,5 @@ resource "juju_offer" "this" {
data "juju_offer" "this" {
url = juju_offer.this.url
}
`, modelName, offerName)
`, modelName, os, offerName)
}
34 changes: 29 additions & 5 deletions internal/provider/resource_application.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"strings"

"github.com/hashicorp/terraform-plugin-framework-validators/listvalidator"
"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/path"
Expand Down Expand Up @@ -198,13 +199,33 @@ func (r *applicationResource) Schema(_ context.Context, _ resource.SchemaRequest
int64planmodifier.UseStateForUnknown(),
},
},
"series": schema.StringAttribute{
SeriesKey: schema.StringAttribute{
Description: "The series on which to deploy.",
Optional: true,
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
Validators: []validator.String{
stringvalidator.ConflictsWith(path.Expressions{
path.MatchRelative().AtParent().AtName(BaseKey),
}...),
},
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].",
Optional: true,
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
Validators: []validator.String{
stringvalidator.ConflictsWith(path.Expressions{
path.MatchRelative().AtParent().AtName(SeriesKey),
}...),
stringIsBaseValidator{},
},
},
},
},
Expand Down Expand Up @@ -245,6 +266,7 @@ type nestedCharm struct {
Name types.String `tfsdk:"name"`
Channel types.String `tfsdk:"channel"`
Revision types.Int64 `tfsdk:"revision"`
Base types.String `tfsdk:"base"`
Series types.String `tfsdk:"series"`
}

Expand Down Expand Up @@ -323,7 +345,6 @@ func (r *applicationResource) Create(ctx context.Context, req resource.CreateReq
if !planCharm.Revision.IsUnknown() {
revision = int(planCharm.Revision.ValueInt64())
}
series := planCharm.Series.ValueString()

// TODO: investigate using map[string]string here and let
// terraform do the conversion, will help in CreateApplication.
Expand Down Expand Up @@ -366,7 +387,8 @@ func (r *applicationResource) Create(ctx context.Context, req resource.CreateReq
CharmName: charmName,
CharmChannel: channel,
CharmRevision: revision,
CharmSeries: series,
CharmBase: planCharm.Base.ValueString(),
CharmSeries: planCharm.Series.ValueString(),
Units: int(plan.UnitCount.ValueInt64()),
Config: configField,
Constraints: parsedConstraints,
Expand Down Expand Up @@ -396,6 +418,7 @@ func (r *applicationResource) Create(ctx context.Context, req resource.CreateReq
plan.Placement = types.StringValue(readResp.Placement)
plan.ApplicationName = types.StringValue(createResp.AppName)
planCharm.Revision = types.Int64Value(int64(readResp.Revision))
planCharm.Base = types.StringValue(readResp.Base)
planCharm.Series = types.StringValue(readResp.Series)
planCharm.Channel = types.StringValue(readResp.Channel)
charmType := req.Config.Schema.GetBlocks()[CharmKey].(schema.ListNestedBlock).NestedObject.Type()
Expand Down Expand Up @@ -478,6 +501,7 @@ func (r *applicationResource) Read(ctx context.Context, req resource.ReadRequest
Name: types.StringValue(response.Name),
Channel: types.StringValue(response.Channel),
Revision: types.Int64Value(int64(response.Revision)),
Base: types.StringValue(response.Base),
Series: types.StringValue(response.Series),
}
charmType := req.State.Schema.GetBlocks()[CharmKey].(schema.ListNestedBlock).NestedObject.Type()
Expand Down Expand Up @@ -602,7 +626,7 @@ func (r *applicationResource) Update(ctx context.Context, req resource.UpdateReq
if !planCharm.Channel.Equal(stateCharm.Channel) {
updateApplicationInput.Channel = planCharm.Channel.ValueString()
}
if !planCharm.Series.Equal(stateCharm.Series) {
if !planCharm.Series.Equal(stateCharm.Series) || !planCharm.Base.Equal(stateCharm.Base) {
// 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
Expand Down Expand Up @@ -743,7 +767,7 @@ func (r *applicationResource) Delete(ctx context.Context, req resource.DeleteReq
return
}
var state applicationResourceModel
// Read Terraform prior state state into the model
// Read Terraform prior state into the model
resp.Diagnostics.Append(req.State.Get(ctx, &state)...)
if resp.Diagnostics.HasError() {
return
Expand Down
Loading

0 comments on commit b55c592

Please sign in to comment.