Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Base for machine resource #317

Merged
merged 4 commits into from
Sep 28, 2023
Merged

Conversation

hmlanigan
Copy link
Member

Description

There are two parts. One separate the concerns of the terraform provider from the internal juju code which uses the juju api.

Two, add Base to the schema for machines, while deprecating Series. Regardless of which is supplied by the user, a base will be provided to the AddMachine api call to juju.

A string validator to cover Bases is also added, its used by machine and will be used for application as well.

Part of the effort to replace Series with Base within the terraform plans for juju as seen in #249

Type of change

  • Change existing resource

QA steps

Here is a plan to test the change. Please set manual machines as well, to ensure no changes there.

provider juju {}

resource "juju_model" "testmodel" {
  name = "machinetest"
}

resource "juju_machine" "my-machine" {
  model = juju_model.testmodel.name
  base = "[email protected]"
}

The Provider does not need to know about details of handling the juju
api such as different structures and bulk api calls. Keep a separation
of concerns.
Series is now deprecated and will be removed in the 1.0.0 release.
Including a new string validator to cover base.

This is a feature request and necessary step to move from juju 2.9 to
juju 3.x.
Juju internals has a base a name@track/risk. However no one else uses
them that way. Usually it's name@track, e.g [email protected]. There are no
other risks for operating systems at this time.

Added a caveat emptor for future people just in case.
Copy link
Contributor

@cderici cderici left a comment

Choose a reason for hiding this comment

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

Code looks good. QA looks good. One interesting observation was that destroying machines seems to taking a bit long when it (correctly) errors, at first I thought we weren't destroying, but then the machine disappeared from the status:

I created a machine with

resource "juju_machine" "my-machine2" {
  model = juju_model.testmodel.name
  // base = "[email protected]"
}

Then added a base, which was good.

Then changed the base to something wrong like ubuntu@15, which prompts recreate in the plan. It does correctly points out the error coming from the api for the second step (i.e. create). It's not super clear that the first step (i.e. destroy) is happening. Then while I was writing this message the machine disappeared suddenly (I was "watch"ing the sttatus).

Just a weird observation fyi, the PR is good to go 👍

@hmlanigan hmlanigan merged commit 96b6ed6 into juju:main Sep 28, 2023
14 checks passed
@hmlanigan hmlanigan deleted the base-in-machine-app branch September 28, 2023 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants