From 19022362bdd60a49bd486fc29e2d2fbb729a095e Mon Sep 17 00:00:00 2001 From: Heather Lanigan Date: Thu, 5 Oct 2023 09:11:46 -0400 Subject: [PATCH 1/6] Update operating system selection for application creation. The operating system needs to be more detailed that current to be accurate. It must take into account juju supported workload operating systems as well. This is challenging as we're using juju 2.9.45 code right now and juju 3.x limits supported workload operating systems. Thus checking the controller model agent version. Include not just the model constraints charm version selection, but also the user defined constraints for the application, e.g. arch. --- go.mod | 2 +- internal/juju/applications.go | 206 +++++++++++++++++++++++----------- 2 files changed, 140 insertions(+), 68 deletions(-) diff --git a/go.mod b/go.mod index f1bc9f3a..d95a372c 100644 --- a/go.mod +++ b/go.mod @@ -19,6 +19,7 @@ require ( github.com/juju/charm/v8 v8.0.6 github.com/juju/clock v1.0.3 github.com/juju/cmd/v3 v3.0.13 + github.com/juju/collections v1.0.2 github.com/juju/errors v1.0.0 github.com/juju/names/v4 v4.0.0 github.com/juju/retry v1.0.0 @@ -109,7 +110,6 @@ require ( github.com/juju/ansiterm v1.0.0 // indirect github.com/juju/blobstore/v2 v2.0.0 // indirect github.com/juju/charmrepo/v6 v6.0.3 // indirect - github.com/juju/collections v1.0.2 // indirect github.com/juju/description/v3 v3.0.15 // indirect github.com/juju/featureflag v1.0.0 // indirect github.com/juju/gnuflag v1.0.0 // indirect diff --git a/internal/juju/applications.go b/internal/juju/applications.go index c94cfbd5..d19a17a2 100644 --- a/internal/juju/applications.go +++ b/internal/juju/applications.go @@ -22,16 +22,20 @@ import ( "github.com/juju/charm/v8" charmresources "github.com/juju/charm/v8/resource" "github.com/juju/clock" + "github.com/juju/collections/set" jujuerrors "github.com/juju/errors" apiapplication "github.com/juju/juju/api/client/application" apicharms "github.com/juju/juju/api/client/charms" apiclient "github.com/juju/juju/api/client/client" apimodelconfig "github.com/juju/juju/api/client/modelconfig" + "github.com/juju/juju/api/client/modelmanager" apiresources "github.com/juju/juju/api/client/resources" + apicommoncharm "github.com/juju/juju/api/common/charm" "github.com/juju/juju/cmd/juju/application/utils" "github.com/juju/juju/core/constraints" "github.com/juju/juju/core/instance" "github.com/juju/juju/core/model" + "github.com/juju/juju/core/series" "github.com/juju/juju/environs/config" "github.com/juju/juju/rpc/params" "github.com/juju/juju/version" @@ -221,11 +225,19 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C return nil, fmt.Errorf("specifying a revision requires a channel for future upgrades") } - modelConstraints, err := modelconfigAPIClient.GetModelConstraints() - if err != nil { - return nil, err + // 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() + if err != nil { + return nil, err + } } - platform, err := utils.DeducePlatform(constraints.Value{}, input.CharmSeries, modelConstraints) + platform, err := utils.DeducePlatform(constraints.Value{}, input.CharmSeries, platformCons) if err != nil { return nil, err } @@ -240,77 +252,21 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C // Charm or bundle has been supplied as a URL so we resolve and // deploy using the store but pass in the origin command line // argument so users can target a specific origin. - resolved, err := charmsAPIClient.ResolveCharms([]apicharms.CharmToResolve{{URL: charmURL, Origin: origin}}) + resolvedURL, resolvedOrigin, supportedSeries, err := resolveCharm(charmsAPIClient, charmURL, origin) if err != nil { return nil, err } - if len(resolved) != 1 { - return nil, fmt.Errorf("expected only one resolution, received %d", len(resolved)) - } - resolvedCharm := resolved[0] - if resolvedCharm.Error != nil { - return nil, resolvedCharm.Error - } - if resolvedCharm.Origin.Type == "bundle" { + if resolvedOrigin.Type == "bundle" { return nil, jujuerrors.NotSupportedf("deploying bundles") } - // Figure out the actual series of the charm - var series string - switch { - case input.CharmSeries != "": - // Explicitly request series. - series = input.CharmSeries - case charmURL.Series != "": - // Series specified in charm URL. - series = charmURL.Series - default: - // First try using the default model series if explicitly set, provided - // it is supported by the charm. - // Get the model config - attrs, err := modelconfigAPIClient.ModelGet() - if err != nil { - return nil, jujuerrors.Wrap(err, errors.New("cannot fetch model settings")) - } - modelConfig, err := config.New(config.NoDefaults, attrs) - if err != nil { - return nil, err - } - - var explicit bool - series, explicit = modelConfig.DefaultSeries() - if explicit { - _, err := charm.SeriesForCharm(series, resolvedCharm.SupportedSeries) - if err == nil { - break - } - } - - // Finally, because we are forced we choose LTS - series = version.DefaultSupportedLTS() - } - - // Select an actually supported series - series, err = charm.SeriesForCharm(series, resolvedCharm.SupportedSeries) + seriesToUse, err := c.seriesToUse(modelconfigAPIClient, input.CharmSeries, resolvedOrigin.Series, set.NewStrings(supportedSeries...)) if err != nil { return nil, err } - // Add the charm to the model - origin = resolvedCharm.Origin.WithSeries(series) - - var deployRevision int - if input.CharmRevision > -1 { - deployRevision = input.CharmRevision - } else { - if origin.Revision != nil { - deployRevision = *origin.Revision - } else { - return nil, errors.New("no origin revision") - } - } - - charmURL = resolvedCharm.URL.WithRevision(deployRevision).WithArchitecture(origin.Architecture).WithSeries(series) + origin = resolvedOrigin.WithSeries(seriesToUse) + charmURL = resolvedURL.WithSeries(seriesToUse) resultOrigin, err := charmsAPIClient.AddCharm(charmURL, origin, false) if err != nil { return nil, err @@ -368,7 +324,7 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C return &CreateApplicationResponse{ AppName: appName, Revision: *origin.Revision, - Series: series, + Series: seriesToUse, }, err } @@ -378,10 +334,111 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C return &CreateApplicationResponse{ AppName: appName, Revision: *origin.Revision, - Series: series, + Series: seriesToUse, }, err } +// supportedWorkloadSeries returns a slice of supported workload series +// depending on the controller agent version. This provider currently +// uses juju 2.9.45 code. However the supported workload series list is +// different between juju 2 and juju 3. Handle that here. +func (c applicationsClient) supportedWorkloadSeries(imageStream string) (set.Strings, error) { + conn, err := c.GetConnection(nil) + if err != nil { + return nil, err + } + + modelManagerAPIClient := modelmanager.NewClient(conn) + defer modelManagerAPIClient.Close() + + uuid, err := c.ModelUUID("controller") + if err != nil { + return nil, err + } + + info, err := modelManagerAPIClient.ModelInfo([]names.ModelTag{names.NewModelTag(uuid)}) + if err != nil { + return nil, err + } + if info[0].Error != nil { + return nil, info[0].Error + } + + supportedSeries, err := series.WorkloadSeries(time.Now(), "", imageStream) + if err != nil { + return nil, err + } + if info[0].Result.AgentVersion.Major > 2 { + unsupported := set.NewStrings("bionic", "trusty", "windows", "xenial", "centos7", "precise") + supportedSeries = supportedSeries.Difference(unsupported) + } + return supportedSeries, nil +} + +// seriesToUse selects a series to deploy a charm with based on the following +// criteria +// - A user specified series must be supported by the charm and a valid juju +// supported workload series. If so, use that, otherwise if an input series +// is provided, return an error. +// - Next check DefaultSeries from model config. If explicitly defined by the +// user, check against charm and juju supported workloads. Use that if in +// both lists. +// - Third check the suggested series against just supported workload series. +// It has already been checked against charm series. +// +// 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) { + attrs, err := modelconfigAPIClient.ModelGet() + if err != nil { + return "", jujuerrors.Wrap(err, errors.New("cannot fetch model settings")) + } + modelConfig, err := config.New(config.NoDefaults, attrs) + if err != nil { + return "", err + } + + supportedWorkloadSeries, err := c.supportedWorkloadSeries(modelConfig.ImageStream()) + if err != nil { + return "", err + } + + // If the inputSeries is supported by the charm and is a supported + // workload series, use that. + if charmSeries.Contains(inputSeries) && supportedWorkloadSeries.Contains(inputSeries) { + return suggestedSeries, nil + } else if inputSeries != "" { + return "", jujuerrors.NewNotSupported(nil, + fmt.Sprintf("series %q either not supported by the charm, or an unsupported juju workload series with the current version of juju.", inputSeries)) + } + + // We can choose from a list of series, supported both as a + // workload series and the by charm. + supportedSeries := charmSeries.Intersection(supportedWorkloadSeries) + + // If a default series is explicitly defined for the model, + // use that if a supportedSeries. + defaultSeries, explicit := modelConfig.DefaultSeries() + if explicit { + useSeries, err := charm.SeriesForCharm(defaultSeries, supportedSeries.Values()) + if err == nil { + return useSeries, nil + } + } + + // If a suggested series is in the supportedSeries list, use it. + useSeries, err := charm.SeriesForCharm(suggestedSeries, supportedSeries.Values()) + if err == nil { + return useSeries, nil + } + + // Note: This DefaultSupportedLTS is specific to juju 2.9.45 + lts := version.DefaultSupportedLTS() + + // Select an actually supported series + return charm.SeriesForCharm(lts, supportedSeries.Values()) +} + // processExpose is a local function that executes an expose request. // If the exposeConfig argument is nil it simply exits. If not, // an expose request is done populating the request arguments with @@ -949,3 +1006,18 @@ func (c applicationsClient) computeSetCharmConfig(input *UpdateApplicationInput, return &toReturn, nil } + +func resolveCharm(charmsAPIClient *apicharms.Client, curl *charm.URL, origin apicommoncharm.Origin) (*charm.URL, apicommoncharm.Origin, []string, error) { + // Charm or bundle has been supplied as a URL so we resolve and + // deploy using the store but pass in the origin command line + // argument so users can target a specific origin. + resolved, err := charmsAPIClient.ResolveCharms([]apicharms.CharmToResolve{{URL: curl, Origin: origin}}) + if err != nil { + return nil, apicommoncharm.Origin{}, []string{}, err + } + if len(resolved) != 1 { + return nil, apicommoncharm.Origin{}, []string{}, fmt.Errorf("expected only one resolution, received %d", len(resolved)) + } + resolvedCharm := resolved[0] + return resolvedCharm.URL, resolvedCharm.Origin, resolvedCharm.SupportedSeries, resolvedCharm.Error +} From 2e9ac9f8a3e7b275288bc6cf167068fd144f9a1e Mon Sep 17 00:00:00 2001 From: Heather Lanigan Date: Thu, 5 Oct 2023 09:28:40 -0400 Subject: [PATCH 2/6] Fix update application related to charm piece. Remove support for changing the operating system during an update phase. The way this was done could inadvertently install a charm with the wrong operating system on existing units during a refresh. Fix updating the charm revision and channel. This must be done with the same operating system and architecture as the original charm was deployed with. Move changing application constraints to before --- internal/juju/applications.go | 90 +++++++++++------------ internal/provider/resource_application.go | 9 ++- 2 files changed, 50 insertions(+), 49 deletions(-) diff --git a/internal/juju/applications.go b/internal/juju/applications.go index d19a17a2..01fb3498 100644 --- a/internal/juju/applications.go +++ b/internal/juju/applications.go @@ -139,17 +139,15 @@ type UpdateApplicationInput struct { ModelName string ModelInfo *params.ModelInfo AppName string - //Channel string // TODO: Unsupported for now - Units *int - Revision *int - Channel string - Series string - Trust *bool - Expose map[string]interface{} + Units *int + Revision *int + Channel string + Trust *bool + Expose map[string]interface{} // Unexpose indicates what endpoints to unexpose Unexpose []string Config map[string]string - //Series string // TODO: Unsupported for now + //Series string // Unsupported today Placement map[string]interface{} Constraints *constraints.Value } @@ -845,6 +843,14 @@ func (c applicationsClient) UpdateApplication(input *UpdateApplicationInput) err } } + if input.Constraints != nil { + err := applicationAPIClient.SetConstraints(input.AppName, *input.Constraints) + if err != nil { + c.Errorf(err, "setting application constraints") + return err + } + } + if input.Units != nil { // TODO: Refactor this to a separate function modelType, err := c.ModelType(input.ModelName) @@ -897,27 +903,19 @@ func (c applicationsClient) UpdateApplication(input *UpdateApplicationInput) err // use the revision, channel, and series info to create the // corresponding SetCharm info - - if input.Revision != nil || input.Channel != "" || input.Series != "" { - setCharmConfig, err := c.computeSetCharmConfig(input, appStatus.Series, appStatus.CharmChannel, applicationAPIClient, modelconfigAPIClient, charmsAPIClient) + if input.Revision != nil || input.Channel != "" { + setCharmConfig, err := c.computeSetCharmConfig(input, applicationAPIClient, charmsAPIClient) if err != nil { return err } + // TODO, empty branch name could further break generations. err = applicationAPIClient.SetCharm("", *setCharmConfig) if err != nil { return err } } - if input.Constraints != nil { - err := applicationAPIClient.SetConstraints(input.AppName, *input.Constraints) - if err != nil { - c.Errorf(err, "setting application constraints") - return err - } - } - return nil } @@ -948,8 +946,12 @@ func (c applicationsClient) DestroyApplication(input *DestroyApplicationInput) e // computeSetCharmConfig populates the corresponding configuration object // to indicate juju what charm to be deployed. -func (c applicationsClient) computeSetCharmConfig(input *UpdateApplicationInput, currentSeries string, currentChannel string, applicationAPIClient *apiapplication.Client, modelconfigAPIClient *apimodelconfig.Client, charmsAPIClient *apicharms.Client) (*apiapplication.SetCharmConfig, error) { - oldURL, _, err := applicationAPIClient.GetCharmURLOrigin("", input.AppName) +func (c applicationsClient) computeSetCharmConfig( + input *UpdateApplicationInput, + applicationAPIClient *apiapplication.Client, + charmsAPIClient *apicharms.Client, +) (*apiapplication.SetCharmConfig, error) { + oldURL, oldOrigin, err := applicationAPIClient.GetCharmURLOrigin("", input.AppName) if err != nil { return nil, err } @@ -959,39 +961,27 @@ func (c applicationsClient) computeSetCharmConfig(input *UpdateApplicationInput, newURL = oldURL.WithRevision(*input.Revision) } - modelConstraints, err := modelconfigAPIClient.GetModelConstraints() - if err != nil { - return nil, err - } - - // if no series were set, we will use the current one or the default - series := input.Series - if series == "" { - series = currentSeries - } - - platform, err := utils.DeducePlatform(constraints.Value{}, series, modelConstraints) - if err != nil { - return nil, err - } - - // if no channel, use the current channel - channel := input.Channel - if channel == "" { - channel = currentChannel - } - - parsedChannel, err := charm.ParseChannel(channel) - if err != nil { - return nil, err + newOrigin := oldOrigin + if input.Channel != "" { + parsedChannel, err := charm.ParseChannel(input.Channel) + if err != nil { + return nil, err + } + if parsedChannel.Track != "" { + newOrigin.Track = strPtr(parsedChannel.Track) + } + newOrigin.Risk = string(parsedChannel.Risk) + if parsedChannel.Branch != "" { + newOrigin.Branch = strPtr(parsedChannel.Branch) + } } - origin, err := utils.DeduceOrigin(newURL, parsedChannel, platform) + resolvedURL, resolvedOrigin, _, err := resolveCharm(charmsAPIClient, newURL, newOrigin) if err != nil { return nil, err } - resultOrigin, err := charmsAPIClient.AddCharm(newURL, origin, false) + resultOrigin, err := charmsAPIClient.AddCharm(resolvedURL, resolvedOrigin, false) if err != nil { return nil, err } @@ -1021,3 +1011,7 @@ func resolveCharm(charmsAPIClient *apicharms.Client, curl *charm.URL, origin api resolvedCharm := resolved[0] return resolvedCharm.URL, resolvedCharm.Origin, resolvedCharm.SupportedSeries, resolvedCharm.Error } + +func strPtr(in string) *string { + return &in +} diff --git a/internal/provider/resource_application.go b/internal/provider/resource_application.go index 88912950..8214a1fb 100644 --- a/internal/provider/resource_application.go +++ b/internal/provider/resource_application.go @@ -603,7 +603,14 @@ func (r *applicationResource) Update(ctx context.Context, req resource.UpdateReq updateApplicationInput.Channel = planCharm.Channel.ValueString() } if !planCharm.Series.Equal(stateCharm.Series) { - updateApplicationInput.Series = planCharm.Series.ValueString() + // 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.") } if !planCharm.Revision.Equal(stateCharm.Revision) { updateApplicationInput.Revision = intPtr(planCharm.Revision) From 50aaa69bad68a1870ae4071d8024cccf87d51c8f Mon Sep 17 00:00:00 2001 From: Heather Lanigan Date: Fri, 6 Oct 2023 14:30:40 -0400 Subject: [PATCH 3/6] Only create juju clients when they will be used. This makes the code a bit faster and uses less resources. Especially do not create unused clients. --- internal/juju/applications.go | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/internal/juju/applications.go b/internal/juju/applications.go index 01fb3498..2badf292 100644 --- a/internal/juju/applications.go +++ b/internal/juju/applications.go @@ -24,6 +24,7 @@ import ( "github.com/juju/clock" "github.com/juju/collections/set" jujuerrors "github.com/juju/errors" + "github.com/juju/juju/api" apiapplication "github.com/juju/juju/api/client/application" apicharms "github.com/juju/juju/api/client/charms" apiclient "github.com/juju/juju/api/client/client" @@ -199,13 +200,6 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C modelconfigAPIClient := apimodelconfig.NewClient(conn) defer modelconfigAPIClient.Close() - resourcesAPIClient, err := apiresources.NewClient(conn) - if err != nil { - return nil, err - } - - defer resourcesAPIClient.Close() - channel, err := charm.ParseChannel(input.CharmChannel) if err != nil { return nil, err @@ -275,7 +269,7 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C Origin: resultOrigin, } - resources, err := c.processResources(charmsAPIClient, resourcesAPIClient, charmID, appName) + resources, err := c.processResources(charmsAPIClient, conn, charmID, appName) if err != nil { return nil, err } @@ -500,7 +494,7 @@ func splitCommaDelimitedList(list string) []string { // processResources is a helper function to process the charm // metadata and request the download of any additional resource. -func (c applicationsClient) processResources(charmsAPIClient *apicharms.Client, resourcesAPIClient *apiresources.Client, charmID apiapplication.CharmID, appName string) (map[string]string, error) { +func (c applicationsClient) processResources(charmsAPIClient *apicharms.Client, conn api.Connection, charmID apiapplication.CharmID, appName string) (map[string]string, error) { charmInfo, err := charmsAPIClient.CharmInfo(charmID.URL.String()) if err != nil { return nil, err @@ -511,6 +505,12 @@ func (c applicationsClient) processResources(charmsAPIClient *apicharms.Client, return nil, nil } + resourcesAPIClient, err := apiresources.NewClient(conn) + if err != nil { + return nil, err + } + defer resourcesAPIClient.Close() + pendingResources := []charmresources.Resource{} for _, v := range charmInfo.Meta.Resources { aux := charmresources.Resource{ @@ -663,7 +663,7 @@ func (c applicationsClient) ReadApplication(input *ReadApplicationInput) (*ReadA return nil, fmt.Errorf("failed to parse charm: %v", err) } - returnedConf, err := applicationAPIClient.Get("master", input.AppName) + returnedConf, err := applicationAPIClient.Get(model.GenerationMaster, input.AppName) if err != nil { return nil, fmt.Errorf("failed to get app configuration %v", err) } @@ -787,9 +787,6 @@ func (c applicationsClient) UpdateApplication(input *UpdateApplicationInput) err clientAPIClient := apiclient.NewClient(conn) defer clientAPIClient.Close() - modelconfigAPIClient := apimodelconfig.NewClient(conn) - defer modelconfigAPIClient.Close() - status, err := clientAPIClient.Status(nil) if err != nil { return err @@ -909,8 +906,7 @@ func (c applicationsClient) UpdateApplication(input *UpdateApplicationInput) err return err } - // TODO, empty branch name could further break generations. - err = applicationAPIClient.SetCharm("", *setCharmConfig) + err = applicationAPIClient.SetCharm(model.GenerationMaster, *setCharmConfig) if err != nil { return err } From 6b709e03ab946d740276a85e67a612902d0ec71b Mon Sep 17 00:00:00 2001 From: Heather Lanigan Date: Fri, 6 Oct 2023 15:44:37 -0400 Subject: [PATCH 4/6] Only call Close() on a connection, not a client. Methods with multiple clients, share a connection. Once a client calls Close(), other clients with the same connection close too, perhaps before the end of the method. --- internal/juju/applications.go | 29 +++++++++--------------- internal/juju/client.go | 2 +- internal/juju/credentials.go | 10 ++++---- internal/juju/integrations.go | 8 +++---- internal/juju/machines.go | 7 +++--- internal/juju/models.go | 23 +++++++++---------- internal/juju/offers.go | 18 ++++++--------- internal/juju/sshKeys.go | 6 ++--- internal/juju/users.go | 11 ++++----- internal/provider/resource_model_test.go | 2 +- 10 files changed, 51 insertions(+), 65 deletions(-) diff --git a/internal/juju/applications.go b/internal/juju/applications.go index 2badf292..ae4549d7 100644 --- a/internal/juju/applications.go +++ b/internal/juju/applications.go @@ -190,15 +190,11 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C if err != nil { return nil, err } + defer func() { _ = conn.Close() }() charmsAPIClient := apicharms.NewClient(conn) - defer charmsAPIClient.Close() - applicationAPIClient := apiapplication.NewClient(conn) - defer applicationAPIClient.Close() - modelconfigAPIClient := apimodelconfig.NewClient(conn) - defer modelconfigAPIClient.Close() channel, err := charm.ParseChannel(input.CharmChannel) if err != nil { @@ -241,6 +237,7 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C if err != nil { return nil, err } + // Charm or bundle has been supplied as a URL so we resolve and // deploy using the store but pass in the origin command line // argument so users can target a specific origin. @@ -259,6 +256,7 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C // Add the charm to the model origin = resolvedOrigin.WithSeries(seriesToUse) charmURL = resolvedURL.WithSeries(seriesToUse) + resultOrigin, err := charmsAPIClient.AddCharm(charmURL, origin, false) if err != nil { return nil, err @@ -297,7 +295,7 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C } } - err = applicationAPIClient.Deploy(apiapplication.DeployArgs{ + args := apiapplication.DeployArgs{ CharmID: charmID, ApplicationName: appName, NumUnits: input.Units, @@ -307,7 +305,9 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C 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 @@ -339,9 +339,9 @@ func (c applicationsClient) supportedWorkloadSeries(imageStream string) (set.Str if err != nil { return nil, err } + defer func() { _ = conn.Close() }() modelManagerAPIClient := modelmanager.NewClient(conn) - defer modelManagerAPIClient.Close() uuid, err := c.ModelUUID("controller") if err != nil { @@ -509,7 +509,6 @@ func (c applicationsClient) processResources(charmsAPIClient *apicharms.Client, if err != nil { return nil, err } - defer resourcesAPIClient.Close() pendingResources := []charmresources.Resource{} for _, v := range charmInfo.Meta.Resources { @@ -588,12 +587,10 @@ func (c applicationsClient) ReadApplication(input *ReadApplicationInput) (*ReadA if err != nil { return nil, err } + defer func() { _ = conn.Close() }() applicationAPIClient := apiapplication.NewClient(conn) - defer applicationAPIClient.Close() - clientAPIClient := apiclient.NewClient(conn) - defer clientAPIClient.Close() apps, err := applicationAPIClient.ApplicationsInfo([]names.ApplicationTag{names.NewApplicationTag(input.AppName)}) if err != nil { @@ -777,15 +774,11 @@ func (c applicationsClient) UpdateApplication(input *UpdateApplicationInput) err if err != nil { return err } + defer func() { _ = conn.Close() }() applicationAPIClient := apiapplication.NewClient(conn) - defer applicationAPIClient.Close() - charmsAPIClient := apicharms.NewClient(conn) - defer charmsAPIClient.Close() - clientAPIClient := apiclient.NewClient(conn) - defer clientAPIClient.Close() status, err := clientAPIClient.Status(nil) if err != nil { @@ -920,9 +913,9 @@ func (c applicationsClient) DestroyApplication(input *DestroyApplicationInput) e if err != nil { return err } + defer func() { _ = conn.Close() }() applicationAPIClient := apiapplication.NewClient(conn) - defer applicationAPIClient.Close() var destroyParams = apiapplication.DestroyApplicationsParams{ Applications: []string{ diff --git a/internal/juju/client.go b/internal/juju/client.go index 57f3bb59..bcc53bc6 100644 --- a/internal/juju/client.go +++ b/internal/juju/client.go @@ -171,9 +171,9 @@ func (sc *sharedClient) fillModelCache() error { if err != nil { return err } + defer func() { _ = conn.Close() }() client := modelmanager.NewClient(conn) - defer func() { _ = client.Close() }() // Calling ListModelSummaries because other Model endpoints require // the UUID, here we're trying to get the model UUID for other calls. diff --git a/internal/juju/credentials.go b/internal/juju/credentials.go index 7e897377..ffe8d8cb 100644 --- a/internal/juju/credentials.go +++ b/internal/juju/credentials.go @@ -89,9 +89,9 @@ func (c *credentialsClient) ValidateCredentialForCloud(cloudName, authTypeReceiv if err != nil { return err } + defer func() { _ = conn.Close() }() client := cloudapi.NewClient(conn) - defer client.Close() cloudTag := names.NewCloudTag(cloudName) @@ -127,9 +127,9 @@ func (c *credentialsClient) CreateCredential(input CreateCredentialInput) (*Crea if err != nil { return nil, err } + defer func() { _ = conn.Close() }() client := cloudapi.NewClient(conn) - defer client.Close() currentUser := getCurrentJujuUser(conn) @@ -170,9 +170,9 @@ func (c *credentialsClient) ReadCredential(input ReadCredentialInput) (*ReadCred if err != nil { return nil, err } + defer func() { _ = conn.Close() }() client := cloudapi.NewClient(conn) - defer client.Close() var clientCredentialFound jujucloud.Credential if clientCredential { @@ -251,6 +251,7 @@ func (c *credentialsClient) UpdateCredential(input UpdateCredentialInput) error if err != nil { return err } + defer func() { _ = conn.Close() }() currentUser := getCurrentJujuUser(conn) @@ -274,7 +275,6 @@ func (c *credentialsClient) UpdateCredential(input UpdateCredentialInput) error if input.ControllerCredential { client := cloudapi.NewClient(conn) - defer client.Close() if _, err := client.UpdateCredentialsCheckModels(*cloudCredTag, cloudCredential); err != nil { return err @@ -318,9 +318,9 @@ func (c *credentialsClient) DestroyCredential(input DestroyCredentialInput) erro if err != nil { return err } + defer func() { _ = conn.Close() }() client := cloudapi.NewClient(conn) - defer client.Close() currentUser := getCurrentJujuUser(conn) diff --git a/internal/juju/integrations.go b/internal/juju/integrations.go index 1367f318..e1b56d73 100644 --- a/internal/juju/integrations.go +++ b/internal/juju/integrations.go @@ -90,9 +90,9 @@ func (c integrationsClient) CreateIntegration(input *IntegrationInput) (*CreateI if err != nil { return nil, err } + defer func() { _ = conn.Close() }() client := apiapplication.NewClient(conn) - defer client.Close() // wait for the apps to be available ctx, cancel := context.WithTimeout(context.Background(), IntegrationAppAvailableTimeout) @@ -130,6 +130,7 @@ func (c integrationsClient) ReadIntegration(input *IntegrationInput) (*ReadInteg if err != nil { return nil, err } + defer func() { _ = conn.Close() }() status, err := getStatus(conn) if err != nil { @@ -190,9 +191,9 @@ func (c integrationsClient) UpdateIntegration(input *UpdateIntegrationInput) (*U if err != nil { return nil, err } + defer func() { _ = conn.Close() }() client := apiapplication.NewClient(conn) - defer client.Close() listViaCIDRs := splitCommaDelimitedList(input.ViaCIDRs) response, err := client.AddRelation( @@ -240,9 +241,9 @@ func (c integrationsClient) DestroyIntegration(input *IntegrationInput) error { if err != nil { return err } + defer func() { _ = conn.Close() }() client := apiapplication.NewClient(conn) - defer client.Close() var force bool = false var timeout time.Duration = 30 * time.Second @@ -261,7 +262,6 @@ func (c integrationsClient) DestroyIntegration(input *IntegrationInput) error { func getStatus(conn api.Connection) (*params.FullStatus, error) { client := apiclient.NewClient(conn) - defer client.Close() status, err := client.Status(nil) if err != nil { diff --git a/internal/juju/machines.go b/internal/juju/machines.go index 19869819..d605c65d 100644 --- a/internal/juju/machines.go +++ b/internal/juju/machines.go @@ -82,12 +82,11 @@ func (c machinesClient) CreateMachine(input *CreateMachineInput) (*CreateMachine if err != nil { return nil, err } + defer func() { _ = conn.Close() }() machineAPIClient := apimachinemanager.NewClient(conn) - defer func() { _ = machineAPIClient.Close() }() modelConfigAPIClient := apimodelconfig.NewClient(conn) - defer func() { _ = modelConfigAPIClient.Close() }() if input.SSHAddress != "" { configAttrs, err := modelConfigAPIClient.ModelGet() @@ -240,9 +239,9 @@ func (c machinesClient) ReadMachine(input ReadMachineInput) (ReadMachineResponse if err != nil { return response, err } + defer func() { _ = conn.Close() }() clientAPIClient := apiclient.NewClient(conn) - defer func() { _ = clientAPIClient.Close() }() status, err := clientAPIClient.Status(nil) if err != nil { @@ -273,9 +272,9 @@ func (c machinesClient) DestroyMachine(input *DestroyMachineInput) error { if err != nil { return err } + defer func() { _ = conn.Close() }() machineAPIClient := apimachinemanager.NewClient(conn) - defer func() { _ = machineAPIClient.Close() }() _, err = machineAPIClient.DestroyMachinesWithParams(false, false, (*time.Duration)(nil), input.ID) diff --git a/internal/juju/models.go b/internal/juju/models.go index 2b40a405..60a2dc59 100644 --- a/internal/juju/models.go +++ b/internal/juju/models.go @@ -102,9 +102,9 @@ func (c *modelsClient) GetModelByName(name string) (*params.ModelInfo, error) { if err != nil { return nil, err } + defer func() { _ = conn.Close() }() client := modelmanager.NewClient(conn) - defer client.Close() modelUUID, err := c.ModelUUID(name) if err != nil { @@ -140,11 +140,11 @@ func (c *modelsClient) CreateModel(input CreateModelInput) (CreateModelResponse, if err != nil { return resp, err } + defer func() { _ = conn.Close() }() currentUser := getCurrentJujuUser(conn) client := modelmanager.NewClient(conn) - defer func() { _ = client.Close() }() cloudName := input.CloudName cloudRegion := input.CloudRegion @@ -189,9 +189,9 @@ func (c *modelsClient) CreateModel(input CreateModelInput) (CreateModelResponse, if err != nil { return resp, err } + defer func() { _ = conn.Close() }() modelClient := modelconfig.NewClient(connModel) - defer func() { _ = modelClient.Close() }() err = modelClient.SetModelConstraints(input.Constraints) if err != nil { return resp, err @@ -205,17 +205,16 @@ func (c *modelsClient) ReadModel(name string) (*ReadModelResponse, error) { if err != nil { return nil, err } + defer func() { _ = modelmanagerConn.Close() }() modelconfigConn, err := c.GetConnection(&name) if err != nil { return nil, errors.Wrap(err, &modelNotFoundError{uuid: name}) } + defer func() { _ = modelconfigConn.Close() }() modelmanagerClient := modelmanager.NewClient(modelmanagerConn) - defer modelmanagerClient.Close() - modelconfigClient := modelconfig.NewClient(modelconfigConn) - defer modelconfigClient.Close() modelUUIDTag, modelOk := modelconfigConn.ModelTag() if !modelOk { @@ -257,9 +256,9 @@ func (c *modelsClient) UpdateModel(input UpdateModelInput) error { if err != nil { return err } + defer func() { _ = conn.Close() }() client := modelconfig.NewClient(conn) - defer client.Close() configMap := make(map[string]interface{}) for key, value := range input.Config { @@ -298,12 +297,12 @@ func (c *modelsClient) UpdateModel(input UpdateModelInput) error { if err != nil { return err } + defer func() { _ = conn.Close() }() modelUUIDTag, modelOk := conn.ModelTag() if !modelOk { return errors.Errorf("Not connected to model %q", input.Name) } clientModelManager := modelmanager.NewClient(connModelManager) - defer clientModelManager.Close() if err := clientModelManager.ChangeModelCredential(modelUUIDTag, *cloudCredTag); err != nil { return err } @@ -317,9 +316,9 @@ func (c *modelsClient) DestroyModel(input DestroyModelInput) error { if err != nil { return err } + defer func() { _ = conn.Close() }() client := modelmanager.NewClient(conn) - defer client.Close() maxWait := 10 * time.Minute timeout := 30 * time.Minute @@ -343,9 +342,9 @@ func (c *modelsClient) GrantModel(input GrantModelInput) error { if err != nil { return err } + defer func() { _ = conn.Close() }() client := modelmanager.NewClient(conn) - defer client.Close() modelUUID, err := c.ModelUUID(input.ModelName) if err != nil { @@ -376,9 +375,9 @@ func (c *modelsClient) UpdateAccessModel(input UpdateAccessModelInput) error { if err != nil { return err } + defer func() { _ = conn.Close() }() client := modelmanager.NewClient(conn) - defer client.Close() for _, user := range input.Revoke { err := client.RevokeModel(user, "read", uuid) @@ -405,9 +404,9 @@ func (c *modelsClient) DestroyAccessModel(input DestroyAccessModelInput) error { if err != nil { return err } + defer func() { _ = conn.Close() }() client := modelmanager.NewClient(conn) - defer func() { _ = client.Close() }() uuid, err := c.ModelUUID(input.ModelName) if err != nil { diff --git a/internal/juju/offers.go b/internal/juju/offers.go index b853c415..b4be70c1 100644 --- a/internal/juju/offers.go +++ b/internal/juju/offers.go @@ -88,9 +88,9 @@ func (c offersClient) CreateOffer(input *CreateOfferInput) (*CreateOfferResponse if err != nil { return nil, append(errs, err) } + defer func() { _ = conn.Close() }() client := applicationoffers.NewClient(conn) - defer client.Close() offerName := input.Name if offerName == "" { @@ -102,9 +102,8 @@ func (c offersClient) CreateOffer(input *CreateOfferInput) (*CreateOfferResponse if err != nil { return nil, append(errs, err) } - defer modelConn.Close() + defer func() { _ = modelConn.Close() }() applicationClient := application.NewClient(modelConn) - defer applicationClient.Close() // wait for the app to be available ctx, cancel := context.WithTimeout(context.Background(), OfferAppAvailableTimeout) @@ -159,10 +158,9 @@ func (c offersClient) ReadOffer(input *ReadOfferInput) (*ReadOfferResponse, erro if err != nil { return nil, err } + defer func() { _ = conn.Close() }() client := applicationoffers.NewClient(conn) - defer client.Close() - result, err := client.ApplicationOffer(input.OfferURL) if err != nil { return nil, err @@ -190,10 +188,9 @@ func (c offersClient) DestroyOffer(input *DestroyOfferInput) error { if err != nil { return err } + defer func() { _ = conn.Close() }() client := applicationoffers.NewClient(conn) - defer client.Close() - offer, err := client.ApplicationOffer(input.OfferURL) if err != nil { return err @@ -259,15 +256,15 @@ func (c offersClient) ConsumeRemoteOffer(input *ConsumeRemoteOfferInput) (*Consu if err != nil { return nil, err } + defer func() { _ = modelConn.Close() }() conn, err := c.GetConnection(nil) if err != nil { return nil, err } + defer func() { _ = conn.Close() }() offersClient := applicationoffers.NewClient(conn) - defer offersClient.Close() client := apiapplication.NewClient(modelConn) - defer client.Close() url, err := crossmodel.ParseOfferURL(input.OfferURL) if err != nil { @@ -342,11 +339,10 @@ func (c offersClient) RemoveRemoteOffer(input *RemoveRemoteOfferInput) []error { errors = append(errors, err) return errors } + defer func() { _ = conn.Close() }() client := apiapplication.NewClient(conn) - defer client.Close() clientAPIClient := apiclient.NewClient(conn) - defer clientAPIClient.Close() status, err := clientAPIClient.Status(nil) if err != nil { diff --git a/internal/juju/sshKeys.go b/internal/juju/sshKeys.go index 9f9e888e..3122ac74 100644 --- a/internal/juju/sshKeys.go +++ b/internal/juju/sshKeys.go @@ -47,9 +47,9 @@ func (c *sshKeysClient) CreateSSHKey(input *CreateSSHKeyInput) error { if err != nil { return err } + defer func() { _ = conn.Close() }() client := keymanager.NewClient(conn) - defer client.Close() // NOTE // Juju only stores ssh keys at a global level. @@ -79,9 +79,9 @@ func (c *sshKeysClient) ReadSSHKey(input *ReadSSHKeyInput) (*ReadSSHKeyOutput, e if err != nil { return nil, err } + defer func() { _ = conn.Close() }() client := keymanager.NewClient(conn) - defer client.Close() // NOTE: At this moment Juju only uses global ssh keys. // We hardcode the user to be admin. @@ -109,9 +109,9 @@ func (c *sshKeysClient) DeleteSSHKey(input *DeleteSSHKeyInput) error { if err != nil { return err } + defer func() { _ = conn.Close() }() client := keymanager.NewClient(conn) - defer client.Close() // NOTE: Unfortunately Juju will return an error if we try to // remove the last ssh key from the controller. This is something diff --git a/internal/juju/users.go b/internal/juju/users.go index 2e0b9051..84784817 100644 --- a/internal/juju/users.go +++ b/internal/juju/users.go @@ -61,9 +61,9 @@ func (c *usersClient) CreateUser(input CreateUserInput) (*CreateUserResponse, er if err != nil { return nil, err } + defer func() { _ = conn.Close() }() client := usermanager.NewClient(conn) - defer client.Close() userTag, userSecret, err := client.AddUser(input.Name, input.DisplayName, input.Password) if err != nil { @@ -78,9 +78,9 @@ func (c *usersClient) ReadUser(name string) (*ReadUserResponse, error) { if err != nil { return nil, err } + defer func() { _ = usermanagerConn.Close() }() usermanagerClient := usermanager.NewClient(usermanagerConn) - defer usermanagerClient.Close() users, err := usermanagerClient.UserInfo([]string{name}, false) //don't list disabled users if err != nil { @@ -106,9 +106,8 @@ func (c *usersClient) ModelUserInfo(modelName string) (*ReadModelUserResponse, e if err != nil { return nil, err } - + defer func() { _ = usermanagerConn.Close() }() usermanagerClient := usermanager.NewClient(usermanagerConn) - defer usermanagerClient.Close() uuid, err := c.ModelUUID(modelName) if err != nil { @@ -134,9 +133,9 @@ func (c *usersClient) UpdateUser(input UpdateUserInput) error { if err != nil { return err } + defer func() { _ = conn.Close() }() client := usermanager.NewClient(conn) - defer client.Close() if input.Password != "" { err = client.SetPassword(input.Name, input.Password) @@ -153,9 +152,9 @@ func (c *usersClient) DestroyUser(input DestroyUserInput) error { if err != nil { return err } + defer func() { _ = conn.Close() }() client := usermanager.NewClient(conn) - defer client.Close() err = client.RemoveUser(input.Name) if err != nil { diff --git a/internal/provider/resource_model_test.go b/internal/provider/resource_model_test.go index c1defee7..6a35b7df 100644 --- a/internal/provider/resource_model_test.go +++ b/internal/provider/resource_model_test.go @@ -226,10 +226,10 @@ func testAccCheckDevelopmentConfigIsUnset(modelName string) resource.TestCheckFu if err != nil { return err } + defer func() { _ = conn.Close() }() // TODO: consider adding to client so we don't expose this layer (even in tests) modelconfigClient := modelconfig.NewClient(conn) - defer modelconfigClient.Close() metadata, err := modelconfigClient.ModelGetWithMetadata() if err != nil { From ab0e464303447ce6bf868cf7b5fcd0bbfce192b8 Mon Sep 17 00:00:00 2001 From: Heather Lanigan Date: Mon, 16 Oct 2023 16:14:22 -0400 Subject: [PATCH 5/6] Anticipate LP 2039179. In juju 2.9.45, ResolveCharms may return a charm and sugested operating system which do not reflect or support the operating system requested by the user. Given an appropriate error to the user if so, rather than continuing to deploy a charm unsupported by the user specified operating system. Update UT to use charms and charm/operating system combinations which will not hit this bug. --- internal/juju/applications.go | 5 ++ internal/provider/data_source_offer_test.go | 2 +- .../provider/resource_integration_test.go | 62 +++++++++---------- internal/provider/resource_offer_test.go | 4 +- 4 files changed, 39 insertions(+), 34 deletions(-) diff --git a/internal/juju/applications.go b/internal/juju/applications.go index ae4549d7..2728fda9 100644 --- a/internal/juju/applications.go +++ b/internal/juju/applications.go @@ -248,11 +248,16 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C if resolvedOrigin.Type == "bundle" { return nil, jujuerrors.NotSupportedf("deploying bundles") } + c.Tracef("heather", map[string]interface{}{"resolved origin": resolvedOrigin, "supported series": supportedSeries}) seriesToUse, err := c.seriesToUse(modelconfigAPIClient, input.CharmSeries, 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. ") + } + // Add the charm to the model origin = resolvedOrigin.WithSeries(seriesToUse) charmURL = resolvedURL.WithSeries(seriesToUse) diff --git a/internal/provider/data_source_offer_test.go b/internal/provider/data_source_offer_test.go index 6f1be2d0..1fdff1af 100644 --- a/internal/provider/data_source_offer_test.go +++ b/internal/provider/data_source_offer_test.go @@ -68,7 +68,7 @@ resource "juju_application" "this" { charm { name = "postgresql" - series = "focal" + series = "jammy" } } diff --git a/internal/provider/resource_integration_test.go b/internal/provider/resource_integration_test.go index ac360415..41e76018 100644 --- a/internal/provider/resource_integration_test.go +++ b/internal/provider/resource_integration_test.go @@ -25,12 +25,12 @@ func TestAcc_ResourceIntegration_Edge(t *testing.T) { CheckDestroy: testAccCheckIntegrationDestroy, Steps: []resource.TestStep{ { - Config: testAccResourceIntegration(modelName, "two"), + Config: testAccResourceIntegration(modelName), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("juju_integration.this", "model", modelName), - resource.TestCheckResourceAttr("juju_integration.this", "id", fmt.Sprintf("%v:%v:%v", modelName, "two:db-admin", "one:backend-db-admin")), + resource.TestCheckResourceAttr("juju_integration.this", "id", fmt.Sprintf("%v:%v:%v", modelName, "one:source", "two:sink")), resource.TestCheckResourceAttr("juju_integration.this", "application.#", "2"), - resource.TestCheckTypeSetElemNestedAttrs("juju_integration.this", "application.*", map[string]string{"name": "one", "endpoint": "backend-db-admin"}), + resource.TestCheckTypeSetElemNestedAttrs("juju_integration.this", "application.*", map[string]string{"name": "one", "endpoint": "source"}), ), }, { @@ -39,10 +39,10 @@ func TestAcc_ResourceIntegration_Edge(t *testing.T) { ResourceName: "juju_integration.this", }, { - Config: testAccResourceIntegration(modelName, "two"), + Config: testAccResourceIntegration(modelName), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("juju_integration.this", "model", modelName), - resource.TestCheckResourceAttr("juju_integration.this", "id", fmt.Sprintf("%v:%v:%v", modelName, "two:db-admin", "one:backend-db-admin")), + resource.TestCheckResourceAttr("juju_integration.this", "id", fmt.Sprintf("%v:%v:%v", modelName, "one:source", "two:sink")), resource.TestCheckResourceAttr("juju_integration.this", "application.#", "2"), ), }, @@ -67,9 +67,9 @@ func TestAcc_ResourceIntegrationWithViaCIDRs_Edge(t *testing.T) { Config: testAccResourceIntegrationWithVia(srcModelName, dstModelName, via), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("juju_integration.a", "model", srcModelName), - resource.TestCheckResourceAttr("juju_integration.a", "id", fmt.Sprintf("%v:%v:%v", srcModelName, "a:db-admin", "b:backend-db-admin")), + resource.TestCheckResourceAttr("juju_integration.a", "id", fmt.Sprintf("%v:%v:%v", srcModelName, "a:source", "b:sink")), resource.TestCheckResourceAttr("juju_integration.a", "application.#", "2"), - resource.TestCheckTypeSetElemNestedAttrs("juju_integration.a", "application.*", map[string]string{"name": "a", "endpoint": "db-admin"}), + resource.TestCheckTypeSetElemNestedAttrs("juju_integration.a", "application.*", map[string]string{"name": "a", "endpoint": "source"}), resource.TestCheckResourceAttr("juju_integration.a", "via", via), ), }, @@ -94,12 +94,12 @@ func TestAcc_ResourceIntegration_Stable(t *testing.T) { CheckDestroy: testAccCheckIntegrationDestroy, Steps: []resource.TestStep{ { - Config: testAccResourceIntegration(modelName, "two"), + Config: testAccResourceIntegration(modelName), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("juju_integration.this", "model", modelName), - resource.TestCheckResourceAttr("juju_integration.this", "id", fmt.Sprintf("%v:%v:%v", modelName, "two:db-admin", "one:backend-db-admin")), + resource.TestCheckResourceAttr("juju_integration.this", "id", fmt.Sprintf("%v:%v:%v", modelName, "one:source", "two:sink")), resource.TestCheckResourceAttr("juju_integration.this", "application.#", "2"), - resource.TestCheckTypeSetElemNestedAttrs("juju_integration.this", "application.*", map[string]string{"name": "one", "endpoint": "backend-db-admin"}), + resource.TestCheckTypeSetElemNestedAttrs("juju_integration.this", "application.*", map[string]string{"name": "one", "endpoint": "source"}), ), }, { @@ -108,10 +108,10 @@ func TestAcc_ResourceIntegration_Stable(t *testing.T) { ResourceName: "juju_integration.this", }, { - Config: testAccResourceIntegration(modelName, "two"), + Config: testAccResourceIntegration(modelName), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("juju_integration.this", "model", modelName), - resource.TestCheckResourceAttr("juju_integration.this", "id", fmt.Sprintf("%v:%v:%v", modelName, "two:db-admin", "one:backend-db-admin")), + resource.TestCheckResourceAttr("juju_integration.this", "id", fmt.Sprintf("%v:%v:%v", modelName, "one:source", "two:sink")), resource.TestCheckResourceAttr("juju_integration.this", "application.#", "2"), ), }, @@ -123,7 +123,7 @@ func testAccCheckIntegrationDestroy(s *terraform.State) error { return nil } -func testAccResourceIntegration(modelName string, integrationName string) string { +func testAccResourceIntegration(modelName string) string { return fmt.Sprintf(` resource "juju_model" "this" { name = %q @@ -134,8 +134,8 @@ resource "juju_application" "one" { name = "one" charm { - name = "pgbouncer" - series = "focal" + name = "juju-qa-dummy-sink" + series = "jammy" } } @@ -144,8 +144,8 @@ resource "juju_application" "two" { name = "two" charm { - name = "postgresql" - series = "focal" + name = "juju-qa-dummy-source" + series = "jammy" } } @@ -153,16 +153,16 @@ resource "juju_integration" "this" { model = juju_model.this.name application { - name = juju_application.%s.name - endpoint = "db-admin" + name = juju_application.one.name + endpoint = "source" } application { - name = juju_application.one.name - endpoint = "backend-db-admin" + name = juju_application.two.name + endpoint = "sink" } } -`, modelName, integrationName) +`, modelName) } func TestAcc_ResourceIntegrationWithViaCIDRs_Stable(t *testing.T) { @@ -187,9 +187,9 @@ func TestAcc_ResourceIntegrationWithViaCIDRs_Stable(t *testing.T) { Config: testAccResourceIntegrationWithVia(srcModelName, dstModelName, via), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("juju_integration.a", "model", srcModelName), - resource.TestCheckResourceAttr("juju_integration.a", "id", fmt.Sprintf("%v:%v:%v", srcModelName, "a:db-admin", "b:backend-db-admin")), + resource.TestCheckResourceAttr("juju_integration.a", "id", fmt.Sprintf("%v:%v:%v", srcModelName, "a:source", "b:sink")), resource.TestCheckResourceAttr("juju_integration.a", "application.#", "2"), - resource.TestCheckTypeSetElemNestedAttrs("juju_integration.a", "application.*", map[string]string{"name": "a", "endpoint": "db-admin"}), + resource.TestCheckTypeSetElemNestedAttrs("juju_integration.a", "application.*", map[string]string{"name": "a", "endpoint": "source"}), resource.TestCheckResourceAttr("juju_integration.a", "via", via), ), }, @@ -198,7 +198,7 @@ func TestAcc_ResourceIntegrationWithViaCIDRs_Stable(t *testing.T) { } // testAccResourceIntegrationWithVia generates a plan where a -// postgresql:db-admin relates to a pgbouncer:backend-db-admin using +// postgresql:source relates to a pgbouncer:backend-source using // and offer of pgbouncer. func testAccResourceIntegrationWithVia(srcModelName string, dstModelName string, viaCIDRs string) string { return fmt.Sprintf(` @@ -211,8 +211,8 @@ resource "juju_application" "a" { name = "a" charm { - name = "postgresql" - series = "focal" + name = "juju-qa-dummy-sink" + series = "jammy" } } @@ -225,15 +225,15 @@ resource "juju_application" "b" { name = "b" charm { - name = "pgbouncer" - series = "focal" + name = "juju-qa-dummy-source" + series = "jammy" } } resource "juju_offer" "b" { model = juju_model.b.name application_name = juju_application.b.name - endpoint = "backend-db-admin" + endpoint = "sink" } resource "juju_integration" "a" { @@ -242,7 +242,7 @@ resource "juju_integration" "a" { application { name = juju_application.a.name - endpoint = "db-admin" + endpoint = "source" } application { diff --git a/internal/provider/resource_offer_test.go b/internal/provider/resource_offer_test.go index b74bfd6d..de48f7bd 100644 --- a/internal/provider/resource_offer_test.go +++ b/internal/provider/resource_offer_test.go @@ -66,7 +66,7 @@ resource "juju_application" "appone" { charm { name = "postgresql" - series = "focal" + series = "jammy" } } @@ -148,7 +148,7 @@ resource "juju_application" "this" { charm { name = "postgresql" - series = "focal" + series = "jammy" } } From 01e8bdbbd18b22425ebc80703692d754199b61a1 Mon Sep 17 00:00:00 2001 From: Heather Lanigan Date: Wed, 18 Oct 2023 10:20:10 -0400 Subject: [PATCH 6/6] Update comment referencing series not used and remove temp debug line --- internal/juju/applications.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/juju/applications.go b/internal/juju/applications.go index 2728fda9..adf28257 100644 --- a/internal/juju/applications.go +++ b/internal/juju/applications.go @@ -248,7 +248,6 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C if resolvedOrigin.Type == "bundle" { return nil, jujuerrors.NotSupportedf("deploying bundles") } - c.Tracef("heather", map[string]interface{}{"resolved origin": resolvedOrigin, "supported series": supportedSeries}) seriesToUse, err := c.seriesToUse(modelconfigAPIClient, input.CharmSeries, resolvedOrigin.Series, set.NewStrings(supportedSeries...)) if err != nil { @@ -896,8 +895,8 @@ func (c applicationsClient) UpdateApplication(input *UpdateApplicationInput) err } } - // use the revision, channel, and series info to create the - // corresponding SetCharm info + // Use the revision and channel info to create the + // corresponding SetCharm info. if input.Revision != nil || input.Channel != "" { setCharmConfig, err := c.computeSetCharmConfig(input, applicationAPIClient, charmsAPIClient) if err != nil {