Adds OCI support for AllocatePublicIP constraint.

manadart · manadart · commit 7bd5a8c60dc8 · 2020-10-21T17:14:27.000+02:00
diff --git a/provider/oci/environ.go b/provider/oci/environ.go
@@ -332,7 +332,6 @@ func (e *Environ) ConstraintsValidator(ctx envcontext.ProviderCallContext) (cons
 		constraints.Container,
 		constraints.VirtType,
 		constraints.Tags,
-		constraints.AllocatePublicIP,
 	}
 
 	validator := constraints.NewValidator()
@@ -606,7 +605,11 @@ func (e *Environ) StartInstance(ctx envcontext.ProviderCallContext, args environ
 		rootDiskSizeGB = MinVolumeSizeMB / 1024
 	}
 
-	assignPublicIp := true
+	allocatePublicIP := true
+	if args.Constraints.HasAllocatePublicIP() {
+		allocatePublicIP = *args.Constraints.AllocatePublicIP
+	}
+
 	bootSource := ociCore.InstanceSourceViaImageDetails{
 		ImageId:             &image,
 		BootVolumeSizeInGBs: &rootDiskSizeGB,
@@ -618,7 +621,7 @@ func (e *Environ) StartInstance(ctx envcontext.ProviderCallContext, args environ
 		Shape:              &spec.InstanceType.Name,
 		CreateVnicDetails: &ociCore.CreateVnicDetails{
 			SubnetId:       network.Id,
-			AssignPublicIp: &assignPublicIp,
+			AssignPublicIp: &allocatePublicIP,
 			DisplayName:    &hostname,
 		},
 		DisplayName: &hostname,
@@ -654,9 +657,11 @@ func (e *Environ) StartInstance(ctx envcontext.ProviderCallContext, args environ
 	displayName := shortenMachineId(machineId, 6)
 
 	if desiredStatus == ociCore.InstanceLifecycleStateRunning {
-		if err := instance.waitForPublicIP(ctx); err != nil {
-			providerCommon.HandleCredentialError(err, ctx)
-			return nil, errors.Trace(err)
+		if allocatePublicIP {
+			if err := instance.waitForPublicIP(ctx); err != nil {
+				providerCommon.HandleCredentialError(err, ctx)
+				return nil, errors.Trace(err)
+			}
 		}
 	}
 
diff --git a/provider/oci/environ_test.go b/provider/oci/environ_test.go
@@ -835,7 +835,9 @@ func (e *environSuite) TestAllRunningInstancesExtraUnrelatedInstance(c *gc.C) {
 	c.Check(len(ids), gc.Equals, 2)
 }
 
-func (e *environSuite) setupLaunchInstanceExpectations(isController bool, tags map[string]string) {
+func (e *environSuite) setupLaunchInstanceExpectations(
+	isController bool, tags map[string]string, publicIP bool, launchInstanceMatcher gomock.Matcher,
+) {
 	inst := ociCore.Instance{
 		AvailabilityDomain: makeStringPointer("fakeZone1"),
 		CompartmentId:      &e.testCompartment,
@@ -849,7 +851,7 @@ func (e *environSuite) setupLaunchInstanceExpectations(isController bool, tags m
 	responseLaunch := ociCore.LaunchInstanceResponse{
 		Instance: inst,
 	}
-	e.compute.EXPECT().LaunchInstance(context.Background(), gomock.Any()).Return(responseLaunch, nil)
+	e.compute.EXPECT().LaunchInstance(context.Background(), launchInstanceMatcher).Return(responseLaunch, nil)
 
 	getInst := inst
 	if isController {
@@ -890,8 +892,12 @@ func (e *environSuite) setupLaunchInstanceExpectations(isController bool, tags m
 			},
 		})
 
-		e.compute.EXPECT().ListVnicAttachments(context.Background(), attachRequest).Return(attachResponse, nil)
-		e.netw.EXPECT().GetVnic(context.Background(), vnicRequest[0]).Return(vnicResponse[0], nil)
+		// These calls are only expected if we assign a public IP.
+		// They occur when polling for the IP after the instance is started.
+		if publicIP {
+			e.compute.EXPECT().ListVnicAttachments(context.Background(), attachRequest).Return(attachResponse, nil)
+			e.netw.EXPECT().GetVnic(context.Background(), vnicRequest[0]).Return(vnicResponse[0], nil)
+		}
 	}
 }
 
@@ -904,7 +910,8 @@ func (e *environSuite) setupEnsureNetworksExpectations(vcnId string, machineTags
 	e.setupListSubnetsExpectations(vcnId, "fakeRouteTableId", machineTags, 1)
 }
 
-func (e *environSuite) setupStartInstanceExpectations(isController bool) {
+func (e *environSuite) setupStartInstanceExpectations(
+	isController bool, publicIP bool, launchInstanceMatcher gomock.Matcher) {
 	vcnId := "fakeVCNId"
 	machineTags := map[string]string{
 		tags.JujuController: testing.ControllerTag.Id(),
@@ -917,14 +924,41 @@ func (e *environSuite) setupStartInstanceExpectations(isController bool) {
 
 	e.setupEnsureNetworksExpectations(vcnId, machineTags)
 	e.setupListImagesExpectations()
-	e.setupLaunchInstanceExpectations(isController, machineTags)
+	e.setupLaunchInstanceExpectations(isController, machineTags, publicIP, launchInstanceMatcher)
 }
 
 func (e *environSuite) TestBootstrap(c *gc.C) {
 	ctrl := e.patchEnv(c)
 	defer ctrl.Finish()
 
-	e.setupStartInstanceExpectations(true)
+	e.setupStartInstanceExpectations(true, true, gomock.Any())
+
+	ctx := envtesting.BootstrapContext(c)
+	_, err := e.env.Bootstrap(ctx, nil,
+		environs.BootstrapParams{
+			ControllerConfig:         testing.FakeControllerConfig(),
+			AvailableTools:           makeToolsList("trusty"),
+			BootstrapSeries:          "trusty",
+			SupportedBootstrapSeries: testing.FakeSupportedJujuSeries,
+		})
+	c.Assert(err, gc.IsNil)
+}
+
+type noPublicIPMatcher struct{}
+
+func (noPublicIPMatcher) Matches(arg interface{}) bool {
+	li := arg.(ociCore.LaunchInstanceRequest)
+	assign := *li.CreateVnicDetails.AssignPublicIp
+	return !assign
+}
+
+func (noPublicIPMatcher) String() string { return "" }
+
+func (e *environSuite) TestBootstrapNoAllocatePublicIP(c *gc.C) {
+	ctrl := e.patchEnv(c)
+	defer ctrl.Finish()
+
+	e.setupStartInstanceExpectations(true, false, noPublicIPMatcher{})
 
 	ctx := envtesting.BootstrapContext(c)
 	_, err := e.env.Bootstrap(ctx, nil,
@@ -933,6 +967,7 @@ func (e *environSuite) TestBootstrap(c *gc.C) {
 			AvailableTools:           makeToolsList("trusty"),
 			BootstrapSeries:          "trusty",
 			SupportedBootstrapSeries: testing.FakeSupportedJujuSeries,
+			BootstrapConstraints:     constraints.MustParse("allocate-public-ip=false"),
 		})
 	c.Assert(err, gc.IsNil)
 }
