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..adf28257 100644 --- a/internal/juju/applications.go +++ b/internal/juju/applications.go @@ -22,16 +22,21 @@ 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" + "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" 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" @@ -135,17 +140,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 } @@ -187,22 +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() - - resourcesAPIClient, err := apiresources.NewClient(conn) - if err != nil { - return nil, err - } - - defer resourcesAPIClient.Close() channel, err := charm.ParseChannel(input.CharmChannel) if err != nil { @@ -221,11 +213,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 } @@ -237,80 +237,30 @@ 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. - 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 } + 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 = 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") - } - } + origin = resolvedOrigin.WithSeries(seriesToUse) + charmURL = resolvedURL.WithSeries(seriesToUse) - charmURL = resolvedCharm.URL.WithRevision(deployRevision).WithArchitecture(origin.Architecture).WithSeries(series) resultOrigin, err := charmsAPIClient.AddCharm(charmURL, origin, false) if err != nil { return nil, err @@ -321,7 +271,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 } @@ -349,7 +299,7 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C } } - err = applicationAPIClient.Deploy(apiapplication.DeployArgs{ + args := apiapplication.DeployArgs{ CharmID: charmID, ApplicationName: appName, NumUnits: input.Units, @@ -359,7 +309,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 @@ -368,7 +320,7 @@ func (c applicationsClient) CreateApplication(input *CreateApplicationInput) (*C return &CreateApplicationResponse{ AppName: appName, Revision: *origin.Revision, - Series: series, + Series: seriesToUse, }, err } @@ -378,10 +330,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 + } + defer func() { _ = conn.Close() }() + + modelManagerAPIClient := modelmanager.NewClient(conn) + + 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 @@ -445,7 +498,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 @@ -456,6 +509,11 @@ func (c applicationsClient) processResources(charmsAPIClient *apicharms.Client, return nil, nil } + resourcesAPIClient, err := apiresources.NewClient(conn) + if err != nil { + return nil, err + } + pendingResources := []charmresources.Resource{} for _, v := range charmInfo.Meta.Resources { aux := charmresources.Resource{ @@ -533,12 +591,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 { @@ -608,7 +664,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) } @@ -722,18 +778,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() - - modelconfigAPIClient := apimodelconfig.NewClient(conn) - defer modelconfigAPIClient.Close() status, err := clientAPIClient.Status(nil) if err != nil { @@ -788,6 +837,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) @@ -838,29 +895,20 @@ 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) + // 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 { return err } - err = applicationAPIClient.SetCharm("", *setCharmConfig) + err = applicationAPIClient.SetCharm(model.GenerationMaster, *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 } @@ -869,9 +917,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{ @@ -891,8 +939,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 } @@ -902,39 +954,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 } @@ -949,3 +989,22 @@ 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 +} + +func strPtr(in string) *string { + return &in +} 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/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_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) 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_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 { 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" } }