Merge pull request juju#16357 from nvinuesa/juju-4694

juju#16357

In the context of adding ARM support for Oracle OCI (juju#16277), we must fist support oracle flexible shapes because the only ARM shape on OCI is a flexible one (https://docs.oracle.com/en-us/iaas/Content/Compute/References/computeshapes.htm#flexible).

In order to support flexible (https://docs.oracle.com/en-us/iaas/Content/Compute/References/computeshapes.htm#flexible) shapes we must pass some configuration `ShapeConfig` (at least the number of cpu-cores) at the moment of launching the instance. This configuration value will be either the user-passed constraint(s) cpu-cores and memory, else a default minimum cpu-cores. We also define a new constant for minimum cpu-cores that is matched on `instances.MatchingInstanceTypes`. 

Also, we cannot pass this `ShapeConfig` value if the selected shape is not flexible so we have to be able to distinct between flexible and non-flexible shapes. This was done by adding `MaxCpuCores` and `MaxMem` on `InstanceType`

_Bonus: some cleanup on tests._

## Checklist

*If an item is not applicable, use `~strikethrough~`.*

- [X] Code style: imports ordered, good names, simple structure, etc
- [X] Comments saying why design decisions were made
- [X] Go unit tests, with comments saying what you're testing
- [ ] ~[Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing~
- [ ] ~[doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~

## QA steps

```
juju bootstrap --debug --config compartment-id=ocid1.compartment.oc1..aaaaaaaanvu2racnlczevenu73dlcf3nokgsjpdkbdgp4xbrz3lb2y4giyjq oci-canonical c
```
check OCIs console and make sure the `VM.Standard1.1` shape was selected as the controller running instance.
 
## Links

**Jira card:** JUJU-[4694]

environs/instances/instancetype.go

@@ -25,6 +25,11 @@ type InstanceType struct {
 	VirtType *string // The type of virtualisation used by the hypervisor, must match the image.
 	CpuPower *uint64
 	Tags     []string
+	// These two values are needed to know the maximum value of cpu and
+	// memory on flexible/custom instances. Currently only supported on
+	// OCI.
+	MaxCpuCores *uint64
+	MaxMem      *uint64
 }
 
 // InstanceTypeNetworking hold relevant information about an instances
@@ -65,13 +70,17 @@ func (itype InstanceType) match(cons constraints.Value) (InstanceType, bool) {
 		return nothing, false
 	}
 	if cons.CpuCores != nil && itype.CpuCores < *cons.CpuCores {
-		return nothing, false
+		if itype.MaxCpuCores == nil || *itype.MaxCpuCores < *cons.CpuCores {
+			return nothing, false
+		}
 	}
 	if cons.CpuPower != nil && itype.CpuPower != nil && *itype.CpuPower < *cons.CpuPower {
 		return nothing, false
 	}
 	if cons.Mem != nil && itype.Mem < *cons.Mem {
-		return nothing, false
+		if itype.MaxMem == nil || *itype.MaxMem < *cons.Mem {
+			return nothing, false
+		}
 	}
 	if cons.RootDisk != nil && itype.RootDisk > 0 && itype.RootDisk < *cons.RootDisk {
 		return nothing, false
@@ -85,8 +94,12 @@ func (itype InstanceType) match(cons constraints.Value) (InstanceType, bool) {
 	return itype, true
 }
 
-// minMemoryHeuristic is the assumed minimum amount of memory (in MB) we prefer in order to run a server (1GB)
-const minMemoryHeuristic = 1024
+const (
+	// MinCpuCores is the assumed minimum CPU cores we prefer in order to run a server.
+	MinCpuCores uint64 = 1
+	// minMemoryHeuristic is the assumed minimum amount of memory (in MB) we prefer in order to run a server (2GB)
+	minMemoryHeuristic uint64 = 2048
+)
 
 // matchingTypesForConstraint returns instance types from allTypes which match cons.
 func matchingTypesForConstraint(allTypes []InstanceType, cons constraints.Value) []InstanceType {
@@ -108,13 +121,19 @@ func MatchingInstanceTypes(allInstanceTypes []InstanceType, region string, cons
 
 	// Rules used to select instance types:
 	// - non memory constraints like cores etc are always honoured
+	// - if no cpu-cores constraint specified, try opinionated default
+	//   with enough cpu cores to run a server.
 	// - if no mem constraint specified and instance-type not specified,
 	//   try opinionated default with enough mem to run a server.
 	// - if no matches and no mem constraint specified, try again and
 	//   return any matching instance with the largest memory
 	origCons := cons
-	if !cons.HasInstanceType() && cons.Mem == nil {
-		minMem := uint64(minMemoryHeuristic)
+	if !cons.HasInstanceType() && !cons.HasCpuCores() {
+		minCpuCores := MinCpuCores
+		cons.CpuCores = &minCpuCores
+	}
+	if !cons.HasInstanceType() && !cons.HasMem() {
+		minMem := minMemoryHeuristic
 		cons.Mem = &minMem
 	}
 	itypes = matchingTypesForConstraint(allInstanceTypes, cons)

environs/instances/instancetype_test.go

@@ -102,6 +102,15 @@ var instanceTypes = []InstanceType{
 		Mem:      61952,
 		Cost:     2400,
 		VirtType: &hvm,
+	}, {
+		Name:        "VM.Standard3.Flex",
+		Arch:        "amd64",
+		CpuCores:    1,
+		CpuPower:    CpuPower(8800),
+		MaxCpuCores: makeUint64Pointer(32),
+		Mem:         6144,
+		MaxMem:      makeUint64Pointer(524288),
+		Cost:        50,
 	},
 }
 
@@ -116,24 +125,31 @@ var getInstanceTypesTest = []struct {
 		about: "cores",
 		cons:  "cores=2",
 		expectedItypes: []string{
-			"c1.medium", "m1.large", "m1.xlarge", "c1.large", "c1.xlarge", "cc1.4xlarge",
-			"cc2.8xlarge",
+			"VM.Standard3.Flex", "m1.large", "m1.xlarge", "c1.large", "c1.xlarge", "cc1.4xlarge", "cc2.8xlarge",
 		},
 	}, {
-		about:          "cpu-power",
-		cons:           "cpu-power=2000",
-		expectedItypes: []string{"c1.xlarge", "cc1.4xlarge", "cc2.8xlarge"},
+		about: "17 cores only the flexible shape",
+		cons:  "cores=17",
+		expectedItypes: []string{
+			"VM.Standard3.Flex",
+		},
+	}, {
+		about: "cpu-power",
+		cons:  "cpu-power=2000",
+		expectedItypes: []string{
+			"VM.Standard3.Flex", "c1.xlarge", "cc1.4xlarge", "cc2.8xlarge",
+		},
 	}, {
 		about: "mem",
 		cons:  "mem=4G",
 		expectedItypes: []string{
-			"m1.large", "m1.xlarge", "c1.xlarge", "cc1.4xlarge", "cc2.8xlarge",
+			"VM.Standard3.Flex", "m1.large", "m1.xlarge", "c1.xlarge", "cc1.4xlarge", "cc2.8xlarge",
 		},
 	}, {
 		about: "root-disk",
 		cons:  "root-disk=16G",
 		expectedItypes: []string{
-			"m1.medium", "m1.large", "m1.xlarge", "c1.xlarge", "cc1.4xlarge", "cc2.8xlarge",
+			"VM.Standard3.Flex", "m1.medium", "m1.large", "m1.xlarge", "c1.xlarge", "cc1.4xlarge", "cc2.8xlarge",
 		},
 	}, {
 		about:          "arches filtered by constraint",
@@ -146,8 +162,8 @@ var getInstanceTypesTest = []struct {
 		cons:  "cores=4",
 		itypesToUse: []InstanceType{
 			{Id: "5", Name: "it-5", Arch: "amd64", Mem: 1024, CpuCores: 2},
-			{Id: "4", Name: "it-4", Arch: "amd64", Mem: 2048, CpuCores: 4},
-			{Id: "3", Name: "it-3", Arch: "amd64", Mem: 1024, CpuCores: 4},
+			{Id: "4", Name: "it-4", Arch: "amd64", Mem: 4096, CpuCores: 4},
+			{Id: "3", Name: "it-3", Arch: "amd64", Mem: 2048, CpuCores: 4},
 			{Id: "2", Name: "it-2", Arch: "amd64", Mem: 256, CpuCores: 4},
 			{Id: "1", Name: "it-1", Arch: "amd64", Mem: 512, CpuCores: 4},
 		},
@@ -157,20 +173,20 @@ var getInstanceTypesTest = []struct {
 		about: "small mem specified, use that even though less than needed for mongodb",
 		cons:  "mem=300M",
 		itypesToUse: []InstanceType{
-			{Id: "3", Name: "it-3", Arch: "amd64", Mem: 2048},
-			{Id: "2", Name: "it-2", Arch: "amd64", Mem: 256},
-			{Id: "1", Name: "it-1", Arch: "amd64", Mem: 512},
+			{Id: "3", Name: "it-3", Arch: "amd64", Mem: 2048, CpuCores: 1},
+			{Id: "2", Name: "it-2", Arch: "amd64", Mem: 256, CpuCores: 1},
+			{Id: "1", Name: "it-1", Arch: "amd64", Mem: 512, CpuCores: 1},
 		},
 		expectedItypes: []string{"it-1", "it-3"},
 	},
 	{
 		about: "mem specified and match found",
 		cons:  "mem=4G arch=amd64",
 		itypesToUse: []InstanceType{
-			{Id: "4", Name: "it-4", Arch: "arm64", Mem: 8096},
-			{Id: "3", Name: "it-3", Arch: "amd64", Mem: 4096},
-			{Id: "2", Name: "it-2", Arch: "amd64", Mem: 2048},
-			{Id: "1", Name: "it-1", Arch: "amd64", Mem: 512},
+			{Id: "4", Name: "it-4", Arch: "arm64", Mem: 8096, CpuCores: 1},
+			{Id: "3", Name: "it-3", Arch: "amd64", Mem: 4096, CpuCores: 1},
+			{Id: "2", Name: "it-2", Arch: "amd64", Mem: 2048, CpuCores: 1},
+			{Id: "1", Name: "it-1", Arch: "amd64", Mem: 512, CpuCores: 1},
 		},
 		expectedItypes: []string{"it-3"},
 	},
@@ -245,8 +261,8 @@ func (s *instanceTypeSuite) TestGetMatchingInstanceTypesErrors(c *gc.C) {
 	_, err = MatchingInstanceTypes(instanceTypes, "test", constraints.MustParse("cores=9000"))
 	c.Check(err, gc.ErrorMatches, `no instance types in test matching constraints "cores=9000"`)
 
-	_, err = MatchingInstanceTypes(instanceTypes, "test", constraints.MustParse("mem=90000M"))
-	c.Check(err, gc.ErrorMatches, `no instance types in test matching constraints "mem=90000M"`)
+	_, err = MatchingInstanceTypes(instanceTypes, "test", constraints.MustParse("mem=900000M"))
+	c.Check(err, gc.ErrorMatches, `no instance types in test matching constraints "mem=900000M"`)
 
 	_, err = MatchingInstanceTypes(instanceTypes, "test", constraints.MustParse("instance-type=dep.medium mem=8G"))
 	c.Check(err, gc.ErrorMatches, `no instance types in test matching constraints "instance-type=dep.medium mem=8192M"`)
@@ -273,6 +289,15 @@ var instanceTypeMatchTests = []struct {
 	{"cpu-power=9001", "cc2.8xlarge", ""},
 	{"mem=1G", "t1.micro", ""},
 	{"arch=arm64", "c1.xlarge", ""},
+
+	// Match on flexible shape against their maximum cpu-cores
+	{"cores=1", "VM.Standard3.Flex", "amd64"},
+	{"cores=16", "VM.Standard3.Flex", "amd64"},
+	{"cores=31", "VM.Standard3.Flex", "amd64"},
+	// Match on flexible shape against their maximum memory
+	{"mem=1G", "VM.Standard3.Flex", "amd64"},
+	{"mem=16G", "VM.Standard3.Flex", "amd64"},
+	{"mem=511G", "VM.Standard3.Flex", "amd64"},
 }
 
 func (s *instanceTypeSuite) TestMatch(c *gc.C) {
@@ -459,3 +484,7 @@ func (s *instanceTypeSuite) TestSortByName(c *gc.C) {
 		c.Check(names, gc.DeepEquals, t.expectedItypes)
 	}
 }
+
+func makeUint64Pointer(val uint64) *uint64 {
+	return &val
+}

provider/azure/environ_test.go

@@ -2093,7 +2093,7 @@ func (s *environSuite) TestInstanceInformation(c *gc.C) {
 	s.sender = s.startInstanceSenders(startInstanceSenderParams{bootstrap: false})
 	types, err := env.InstanceTypes(s.callCtx, constraints.Value{})
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(types.InstanceTypes, gc.HasLen, 6)
+	c.Assert(types.InstanceTypes, gc.HasLen, 4)
 
 	cons := constraints.MustParse("mem=4G")
 	types, err = env.InstanceTypes(s.callCtx, cons)

provider/ec2/image_test.go

@@ -53,6 +53,7 @@ var testInstanceTypes = []instances.InstanceType{{
 	Cost:     21,
 }, {
 	Name:     "t3a.medium",
+	CpuCores: 2,
 	CpuPower: instances.CpuPower(700),
 	Mem:      4096,
 	Arch:     "amd64",
@@ -120,7 +121,7 @@ var findInstanceSpecTests = []struct {
 	{
 		version: "18.04",
 		arch:    "amd64",
-		itype:   "t3a.micro",
+		itype:   "t3a.small",
 		image:   "ami-00001133",
 	}, {
 		version: "18.04",
@@ -144,13 +145,13 @@ var findInstanceSpecTests = []struct {
 		version: "18.04",
 		arch:    "amd64",
 		cons:    "mem=",
-		itype:   "t3a.nano",
+		itype:   "t3a.small",
 		image:   "ami-00001133",
 	}, {
 		version: "18.04",
 		arch:    "amd64",
 		cons:    "cpu-power=",
-		itype:   "t3a.micro",
+		itype:   "t3a.small",
 		image:   "ami-00001133",
 	}, {
 		version: "18.04",
@@ -194,13 +195,13 @@ var findInstanceSpecTests = []struct {
 	}, {
 		version: "22.04",
 		arch:    "amd64",
-		itype:   "t3a.micro",
+		itype:   "t3a.small",
 		image:   "ami-02204133",
 	}, {
 		version: "20.04",
 		arch:    "amd64",
 		cons:    "arch=amd64",
-		itype:   "t3a.micro",
+		itype:   "t3a.small",
 		image:   "ami-02004133",
 	}, {
 		version: "20.04",

provider/gce/environ_instance_test.go

@@ -216,7 +216,7 @@ func (s *environInstSuite) TestListMachineTypes(c *gc.C) {
 	// If no zone is specified, no machine types will be pulled.
 	s.FakeConn.Zones = nil
 	_, err := s.Env.InstanceTypes(s.CallCtx, constraints.Value{})
-	c.Assert(err, gc.ErrorMatches, "no instance types in  matching constraints \"cores=2 mem=2048M\"")
+	c.Assert(err, gc.ErrorMatches, "no instance types in  matching constraints.*")
 
 	// If a non-empty list of zones is specified , we will make an API call
 	// to fetch the available machine types.

provider/gce/instance_information.go

@@ -25,17 +25,18 @@ var (
 
 	// minCpuCores is the assumed minimum CPU cores we prefer in order to run a server.
 	minCpuCores uint64 = 2
-
-	// minMemoryHeuristic is the assumed minimum amount of memory (in MB) we prefer in order to run a server (2GB)
-	minMemoryHeuristic uint64 = 2048
 )
 
+// ensureDefaultConstraints adds the minimum number of cpu cores value to the
+// constraints if the user has not provided a cpu-cores constraint.
+// This function exists only so that the minimum cpu cores takes precedence
+// over the default cpu-cores and memory values implemented in
+// instances.MatchingInstanceTypes()
 func ensureDefaultConstraints(c constraints.Value) constraints.Value {
-	if c.HasInstanceType() || c.HasCpuCores() || c.HasMem() {
+	if c.HasInstanceType() || c.HasCpuCores() {
 		return c
 	}
 	c.CpuCores = &minCpuCores
-	c.Mem = &minMemoryHeuristic
 	return c
 }
 

provider/gce/instance_information_test.go

@@ -55,7 +55,7 @@ func (s *instanceInformationSuite) TestEnsureDefaultConstraints(c *gc.C) {
 	cons := constraints.Value{}
 	c.Assert(cons.String(), gc.Equals, ``)
 	cons = ensureDefaultConstraints(cons)
-	c.Assert(cons.String(), gc.Equals, `cores=2 mem=2048M`)
+	c.Assert(cons.String(), gc.Equals, `cores=2`)
 
 	var err error
 	// Do not fill default cores and mem if instance type is provided.
@@ -84,5 +84,5 @@ func (s *instanceInformationSuite) TestEnsureDefaultConstraints(c *gc.C) {
 	cons, err = constraints.Parse(`mem=4096M`)
 	c.Assert(err, jc.ErrorIsNil)
 	cons = ensureDefaultConstraints(cons)
-	c.Assert(cons.String(), gc.Equals, `mem=4096M`)
+	c.Assert(cons.String(), gc.Equals, `cores=2 mem=4096M`)
 }

provider/oci/common_test.go → provider/oci/common_integration_test.go

@@ -231,8 +231,31 @@ func listShapesResponse() []ociCore.Shape {
 			BillingType:                "PAID",
 		},
 		{
-			Shape:                      makeStringPointer("VM.Standard.A1.Flex"),
-			ProcessorDescription:       makeStringPointer("3.0 GHz Ampere® Altra™"),
+			Shape:                makeStringPointer("VM.Standard.A1.Flex"),
+			ProcessorDescription: makeStringPointer("3.0 GHz Ampere® Altra™"),
+			OcpuOptions: &ociCore.ShapeOcpuOptions{
+				Max: makeFloat32Pointer(80),
+			},
+			MemoryOptions: &ociCore.ShapeMemoryOptions{
+				MaxInGBs: makeFloat32Pointer(512),
+			},
+			Ocpus:                      makeFloat32Pointer(1),
+			MemoryInGBs:                makeFloat32Pointer(6),
+			LocalDisks:                 makeIntPointer(0),
+			LocalDisksTotalSizeInGBs:   (*float32)(nil),
+			PlatformConfigOptions:      (*ociCore.ShapePlatformConfigOptions)(nil),
+			IsBilledForStoppedInstance: makeBoolPointer(false),
+			BillingType:                "LIMITED_FREE",
+		},
+		{
+			Shape:                makeStringPointer("VM.Standard3.Flex"),
+			ProcessorDescription: makeStringPointer("2.0 GHz Intel® Xeon® Platinum 8167M (Skylake)"),
+			OcpuOptions: &ociCore.ShapeOcpuOptions{
+				Max: makeFloat32Pointer(32),
+			},
+			MemoryOptions: &ociCore.ShapeMemoryOptions{
+				MaxInGBs: makeFloat32Pointer(512),
+			},
 			Ocpus:                      makeFloat32Pointer(1),
 			MemoryInGBs:                makeFloat32Pointer(6),
 			LocalDisks:                 makeIntPointer(0),

provider/oci/environ.go

@@ -617,6 +617,8 @@ func (e *Environ) startInstance(
 		FreeformTags: tags,
 	}
 
+	ensureShapeConfig(spec.InstanceType, args.Constraints, &instanceDetails)
+
 	request := ociCore.LaunchInstanceRequest{
 		LaunchInstanceDetails: instanceDetails,
 	}
@@ -653,6 +655,35 @@ func (e *Environ) startInstance(
 	return result, nil
 }
 
+func ensureShapeConfig(
+	instanceSpec instances.InstanceType,
+	constraints constraints.Value,
+	instanceDetails *ociCore.LaunchInstanceDetails) {
+
+	// If the selected spec is a flexible shape, we must provide the number
+	// of OCPUs at least, so if the user hasn't provided cpu constraints we
+	// must pass the default value.
+	if (instanceSpec.MaxCpuCores != nil && instanceSpec.MaxCpuCores != &instanceSpec.CpuCores) ||
+		(instanceSpec.MaxMem != nil && instanceSpec.MaxMem != &instanceSpec.Mem) {
+		instanceDetails.ShapeConfig = &ociCore.LaunchInstanceShapeConfigDetails{}
+		if constraints.HasCpuCores() {
+			cpuCores := float32(*constraints.CpuCores)
+			instanceDetails.ShapeConfig.Ocpus = &cpuCores
+		} else {
+			cpuCores := float32(instances.MinCpuCores)
+			instanceDetails.ShapeConfig.Ocpus = &cpuCores
+		}
+		// If we don't set the memory on ShapeConfig, OCI uses a
+		// default value of memory per Ocpu core. For example, for the
+		// VM.Standard.A1.Flex, if we set 2 Ocpus OCI will set 12GB of
+		// memory (default is 6GB per core).
+		if constraints.HasMem() {
+			mem := float32(*constraints.Mem / 1024)
+			instanceDetails.ShapeConfig.MemoryInGBs = &mem
+		}
+	}
+}
+
 // StopInstances implements environs.InstanceBroker.
 func (e *Environ) StopInstances(ctx envcontext.ProviderCallContext, ids ...instance.Id) error {
 	ociInstances, err := e.getOciInstances(ctx, ids...)