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

schema: deprecate placement and introduce machines in the juju_application resource #695

Open
alesstimec opened this issue Mar 12, 2025 · 7 comments
Assignees
Labels
area/schema Review schema changes before work begins kind/feature suggests new feature or enhancement state/untriaged untriaged bug report

Comments

@alesstimec
Copy link
Member

alesstimec commented Mar 12, 2025

Type of change

Changing existing schema

Description

I propose a change to the schema of the juju_application resource.

It currently contains a placement field, which contains the placement directive for the application. It is a string that contains a comma separated list of machines/containers upon which application is to be deployed. It is also closely (and confusingly) tied to the units field - the number of placement targets in the placement field may not be lower than the number of units, but we may request more units of the application to be created than there are placement directives.

I propose that we deprecate the placement field and replace it with machines field, which would contain a list of machine IDs upon which application is to be deployed. The new machines key would be mutually exclusive with both the placement and the units fields.

The confusion around placement is the source of bugs. See #443

Details

Proposed schema for the machines field

"machines": schema.SetAttribute{
	ElementType: types.StringType,
	Description: "Specify the target machines for the application's units. Changing this value" +
		" will cause the application to be destroyed and recreated by terraform.",
	Optional: true,
	Computed: true,
	PlanModifiers: []planmodifier.Set{
		setplanmodifier.RequiresReplaceIfConfigured(),
		setplanmodifier.UseStateForUnknown(),
	},
	Validators: []validator.Set{
		setvalidator.ConflictsWith(path.Expressions{
			path.MatchRoot("placement"),
			path.MatchRoot("units"),
		}...),
	},
},

Example terraform plan

terraform {
  required_providers {
    juju = {
      version = "0.18.0"
      source  = "juju/juju"
    }
  }
}

provider "juju" {}

resource "juju_model" "test" {
  name = "test-model"
}


resource "juju_machine" "machine1" {
  model       = resource.juju_model.test.name
  name        = "testmachine1"
  base        = "ubuntu@22.04"
}

resource "juju_machine" "machine2" {
  model       = resource.juju_model.test.name
  name        = "testmachine2"
  base        = "ubuntu@22.04"
}


resource "juju_application" "app1" {
  name = "app1"

  model = resource.juju_model.test.name

  charm {
    name     = "ubuntu-lite"
    base     = "ubuntu@22.04"
  }

  machines = [juju_machine.machine1.machine_id, juju_machine.machine2.machine_id]
}

Example of migration

If you have a plan like

terraform {
  required_providers {
    juju = {
      version = "0.18.0"
      source  = "juju/juju"
    }
  }
}

provider "juju" {}

resource "juju_model" "test" {
  name = "test-model"
}


resource "juju_machine" "machine1" {
  model       = resource.juju_model.test.name
  name        = "testmachine1"
  base        = "ubuntu@22.04"
}

resource "juju_machine" "machine2" {
  model       = resource.juju_model.test.name
  name        = "testmachine2"
  base        = "ubuntu@22.04"
}


resource "juju_application" "app1" {
  name = "app1"

  model = resource.juju_model.test.name

  charm {
    name     = "ubuntu-lite"
    base     = "ubuntu@22.04"
  }

  units = 2
  placement = "${join(",", sort([juju_machine.machine1.machine_id, juju_machine.machine2.machine_id]))}"
}

you can simply migrate to the the use of the machines field by removing units and placement and adding

machines = [juju_machine.machine1.machine_id, juju_machine.machine2.machine_id]

to the juju_application resource.

Notes & References

No response

@alesstimec alesstimec added area/schema Review schema changes before work begins kind/feature suggests new feature or enhancement state/untriaged untriaged bug report labels Mar 12, 2025
@kian99
Copy link
Contributor

kian99 commented Mar 12, 2025

A set (of strings) is better than a comma separated string.

Could you also consider what plans would break with this change (or will become deprecated) and add a section to show how they should change to adopt the new machines directive.

@SimoneDutto
Copy link
Contributor

Some questions:

first

Optional: true,
Computed: true

So basically it's either set by the user otherwise we compute a default placement? Maybe Computed is not needed here.

second

This is the scenario for IAAS right? For CAAS you can't select the machine, i think we have a validator for that and we have a field on application called model_type you can check.

third

machines should match in some way units?
I can see we have 2 machines and 1 units -> what do we do? Randomly select a machine? Always the first one? This should be clear to the user.

Plus, can you add the reason why we are deprecating placement? Like, "it produces bugs with this and that, etc"

@alesstimec
Copy link
Member Author

machines should match in some way units?

units and machines are mutually exclusive and the number of units will be computed based on the number of machines in the set.

@alesstimec
Copy link
Member Author

second

This is the scenario for IAAS right? For CAAS you can't select the machine, i think we have a validator for that and we have a field on application called model_type you can check.

I don't think we can validate the model type from the plan alone. We need to have access to an actual model to be able to validate this..

@SimoneDutto
Copy link
Contributor

second

This is the scenario for IAAS right? For CAAS you can't select the machine, i think we have a validator for that and we have a field on application called model_type you can check.

I don't think we can validate the model type from the plan alone. We need to have access to an actual model to be able to validate this..

maybe we can do something similar to what i did for the base attribute. It should be a custom plan modifier.

@SimoneDutto
Copy link
Contributor

machines should match in some way units?

units and machines are mutually exclusive and the number of units will be computed based on the number of machines in the set.

if i remove a machine from the list will it scale down without replacing the application?

@alesstimec
Copy link
Member Author

machines should match in some way units?

units and machines are mutually exclusive and the number of units will be computed based on the number of machines in the set.

if i remove a machine from the list will it scale down without replacing the application?

yes, that should be possible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/schema Review schema changes before work begins kind/feature suggests new feature or enhancement state/untriaged untriaged bug report
Projects
None yet
Development

No branches or pull requests

5 participants