chore: add String() method to instance.Id

nvinuesa · nvinuesa · commit 4a4eead8700a · 2024-10-14T14:55:46.000+02:00
diff --git a/apiserver/common/instanceidgetter.go b/apiserver/common/instanceidgetter.go
@@ -69,7 +69,7 @@ func (ig *InstanceIdGetter) InstanceId(ctx context.Context, args params.Entities
 			result.Results[i].Error = apiservererrors.ServerError(err)
 			continue
 		}
-		result.Results[i].Result = string(instanceId)
+		result.Results[i].Result = instanceId.String()
 	}
 	return result, nil
 }
diff --git a/apiserver/common/machine.go b/apiserver/common/machine.go
@@ -168,7 +168,7 @@ func ModelMachineInfo(ctx context.Context, st ModelManagerBackend, machineServic
 		instanceID, displayName, err := machineService.InstanceIDAndName(ctx, machineUUID)
 		switch {
 		case err == nil:
-			mInfo.InstanceId = string(instanceID)
+			mInfo.InstanceId = instanceID.String()
 			mInfo.DisplayName = displayName
 		case errors.Is(err, machineerrors.NotProvisioned):
 			// ok, but no instance ID to get.
diff --git a/apiserver/facades/agent/provisioner/provisioner.go b/apiserver/facades/agent/provisioner/provisioner.go
@@ -626,7 +626,7 @@ func (api *ProvisionerAPI) commonServiceInstances(ctx context.Context, st *state
 			if err != nil && !errors.Is(err, machineerrors.NotProvisioned) {
 				return nil, err
 			}
-			instanceIdSet.Add(string(instanceId))
+			instanceIdSet.Add(instanceId.String())
 		}
 	}
 	instanceIds := make([]instance.Id, instanceIdSet.Size())
diff --git a/apiserver/facades/agent/storageprovisioner/storageprovisioner.go b/apiserver/facades/agent/storageprovisioner/storageprovisioner.go
@@ -814,7 +814,7 @@ func (s *StorageProvisionerAPIv4) VolumeParams(ctx context.Context, args params.
 				VolumeTag:  tag.String(),
 				MachineTag: volumeAttachment.Host().String(),
 				VolumeId:   "",
-				InstanceId: string(instanceId),
+				InstanceId: instanceId.String(),
 				Provider:   volumeParams.Provider,
 				ReadOnly:   volumeAttachmentParams.ReadOnly,
 			}
@@ -1067,7 +1067,7 @@ func (s *StorageProvisionerAPIv4) VolumeAttachmentParams(
 			VolumeTag:  volumeAttachment.Volume().String(),
 			MachineTag: volumeAttachment.Host().String(),
 			VolumeId:   volumeId,
-			InstanceId: string(instanceId),
+			InstanceId: instanceId.String(),
 			Provider:   string(providerType),
 			ReadOnly:   readOnly,
 		}, nil
@@ -1156,7 +1156,7 @@ func (s *StorageProvisionerAPIv4) FilesystemAttachmentParams(
 			FilesystemTag: filesystemAttachment.Filesystem().String(),
 			MachineTag:    hostTag.String(),
 			FilesystemId:  filesystemId,
-			InstanceId:    string(instanceId),
+			InstanceId:    instanceId.String(),
 			Provider:      string(providerType),
 			// TODO(axw) dealias MountPoint. We now have
 			// Path, MountPoint and Location in different
diff --git a/core/instance/instance.go b/core/instance/instance.go
@@ -17,5 +17,10 @@ type Status struct {
 	Message string
 }
 
+// String returns the [Id] as a string.
+func (id Id) String() string {
+	return string(id)
+}
+
 // UnknownId can be used to explicitly specify the instance Id when it does not matter.
 const UnknownId Id = ""
diff --git a/domain/credential/service/modelcredential.go b/domain/credential/service/modelcredential.go
@@ -200,7 +200,7 @@ func checkMachineInstances(ctx stdcontext.Context, machineState MachineState, ma
 
 	instanceIds := set.NewStrings()
 	for _, instance := range instances {
-		id := string(instance.Id())
+		id := instance.Id().String()
 		instanceIds.Add(id)
 		if checkCloudInstances {
 			if _, found := machinesByInstance[id]; !found {
diff --git a/domain/machine/modelmigration/export.go b/domain/machine/modelmigration/export.go
@@ -84,7 +84,7 @@ func (e *exportOperation) Execute(ctx context.Context, model description.Model)
 			return errors.Errorf("retrieving instance ID for machine %q: %w", machineName, err)
 		}
 		instanceArgs := description.CloudInstanceArgs{
-			InstanceId: string(instanceID),
+			InstanceId: instanceID.String(),
 		}
 		hardwareCharacteristics, err := e.service.HardwareCharacteristics(ctx, machineUUID)
 		if errors.Is(err, machineerrors.NotProvisioned) {
diff --git a/domain/machine/state/machine_cloud_instance.go b/domain/machine/state/machine_cloud_instance.go
@@ -146,7 +146,7 @@ WHERE  availability_zone.name = $availabilityZoneName.name
 	return db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error {
 		instanceData := instanceData{
 			MachineUUID: machineUUID,
-			InstanceID:  string(instanceID),
+			InstanceID:  instanceID.String(),
 			DisplayName: displayName,
 		}
 		if hardwareCharacteristics != nil {
diff --git a/domain/modelmigration/service/service.go b/domain/modelmigration/service/service.go
@@ -152,7 +152,7 @@ func (s *Service) CheckMachines(
 	// Build the sets of provider instance IDs and model machine instance IDs.
 	providerInstanceIDsSet := make(set.Strings, len(providerInstances))
 	for _, instance := range providerInstances {
-		providerInstanceIDsSet.Add(string(instance.Id()))
+		providerInstanceIDsSet.Add(instance.Id().String())
 	}
 
 	instanceIDsSet, err := s.st.GetAllInstanceIDs(ctx)
diff --git a/internal/container/broker/broker.go b/internal/container/broker/broker.go
@@ -139,7 +139,7 @@ func releaseContainerAddresses(
 	namespace instance.Namespace,
 	log corelogger.Logger,
 ) {
-	containerTag, err := namespace.MachineTag(string(instanceID))
+	containerTag, err := namespace.MachineTag(instanceID.String())
 	if err != nil {
 		// Not a reason to cause StopInstances to fail though..
 		log.Warningf("unexpected container tag %q: %v", instanceID, err)
diff --git a/internal/provider/ec2/environ.go b/internal/provider/ec2/environ.go
@@ -1717,7 +1717,7 @@ func (e *environ) AdoptResources(ctx envcontext.ProviderCallContext, controllerU
 
 	resourceIds := make([]string, len(instances))
 	for i, instance := range instances {
-		resourceIds[i] = string(instance.Id())
+		resourceIds[i] = instance.Id().String()
 	}
 	groupIds := make([]string, len(groups))
 	for i, g := range groups {
diff --git a/internal/provider/gce/disks.go b/internal/provider/gce/disks.go
@@ -439,7 +439,7 @@ func (v *volumeSource) AttachVolumes(ctx envcontext.ProviderCallContext, attachP
 			mode = google.ModeRW
 		}
 		instanceId := attachment.InstanceId
-		attached, err := v.attachOneVolume(ctx, volumeName, mode, string(instanceId))
+		attached, err := v.attachOneVolume(ctx, volumeName, mode, instanceId.String())
 		if err != nil {
 			logger.Errorf("could not attach %q to %q: %v", volumeName, instanceId, err)
 			results[i].Error = err
diff --git a/internal/provider/maas/util.go b/internal/provider/maas/util.go
@@ -20,7 +20,7 @@ import (
 // extractSystemId extracts the 'system_id' part from an InstanceId.
 // "/MAAS/api/1.0/nodes/system_id/" => "system_id"
 func extractSystemId(instanceId instance.Id) string {
-	trimmed := strings.TrimRight(string(instanceId), "/")
+	trimmed := strings.TrimRight(instanceId.String(), "/")
 	split := strings.Split(trimmed, "/")
 	return split[len(split)-1]
 }
diff --git a/internal/provider/oci/storage_volumes.go b/internal/provider/oci/storage_volumes.go
@@ -353,7 +353,7 @@ func (v *volumeSource) ValidateVolumeParams(params storage.VolumeParams) error {
 }
 
 func (v *volumeSource) volumeAttachments(instanceId instance.Id) ([]ociCore.IScsiVolumeAttachment, error) {
-	instId := string(instanceId)
+	instId := instanceId.String()
 
 	attachments, err := v.computeAPI.ListVolumeAttachments(context.Background(), v.env.ecfg().compartmentID(), &instId)
 	if err != nil {
diff --git a/internal/provider/openstack/provider.go b/internal/provider/openstack/provider.go
@@ -1848,7 +1848,7 @@ func (e *Environ) AdoptResources(ctx envcontext.ProviderCallContext, controllerU
 		err := e.TagInstance(ctx, instance.Id(), controllerTag)
 		if err != nil {
 			logger.Errorf("error updating controller tag for instance %s: %v", instance.Id(), err)
-			failed = append(failed, string(instance.Id()))
+			failed = append(failed, instance.Id().String())
 			if denied := common.MaybeHandleCredentialError(IsAuthorisationFailure, err, ctx); denied {
 				// If we have an invvalid credential, there is no need to proceed: we'll fail 100%.
 				break
diff --git a/internal/provisionertask/provisioner_task.go b/internal/provisionertask/provisioner_task.go
@@ -1528,7 +1528,7 @@ func (task *provisionerTask) doStartMachine(
 	// Gather the charm LXD profile names, including the lxd profile names from
 	// the container brokers.
 	charmLXDProfiles, err := task.gatherCharmLXDProfiles(
-		string(instanceID), machine.Tag().Id(), startInstanceParams.CharmLXDProfiles)
+		instanceID.String(), machine.Tag().Id(), startInstanceParams.CharmLXDProfiles)
 	if err != nil {
 		return errors.Trace(err)
 	}

Original file line number	Diff line number	Diff line change
`@@ -69,7 +69,7 @@ func (ig *InstanceIdGetter) InstanceId(ctx context.Context, args params.Entities`
`69`	`69`	`result.Results[i].Error = apiservererrors.ServerError(err)`
`70`	`70`	`continue`
`71`	`71`	`}`
`72`		`- result.Results[i].Result = string(instanceId)`
	`72`	`+ result.Results[i].Result = instanceId.String()`
`73`	`73`	`}`
`74`	`74`	`return result, nil`
`75`	`75`	`}`
Original file line number	Diff line number	Diff line change
`@@ -626,7 +626,7 @@ func (api ProvisionerAPI) commonServiceInstances(ctx context.Context, st state`
`626`	`626`	`if err != nil && !errors.Is(err, machineerrors.NotProvisioned) {`
`627`	`627`	`return nil, err`
`628`	`628`	`}`
`629`		`- instanceIdSet.Add(string(instanceId))`
	`629`	`+ instanceIdSet.Add(instanceId.String())`
`630`	`630`	`}`
`631`	`631`	`}`
`632`	`632`	`instanceIds := make([]instance.Id, instanceIdSet.Size())`
Original file line number	Diff line number	Diff line change
`@@ -84,7 +84,7 @@ func (e *exportOperation) Execute(ctx context.Context, model description.Model)`
`84`	`84`	`return errors.Errorf("retrieving instance ID for machine %q: %w", machineName, err)`
`85`	`85`	`}`
`86`	`86`	`instanceArgs := description.CloudInstanceArgs{`
`87`		`- InstanceId: string(instanceID),`
	`87`	`+ InstanceId: instanceID.String(),`
`88`	`88`	`}`
`89`	`89`	`hardwareCharacteristics, err := e.service.HardwareCharacteristics(ctx, machineUUID)`
`90`	`90`	`if errors.Is(err, machineerrors.NotProvisioned) {`
Original file line number	Diff line number	Diff line change
`@@ -146,7 +146,7 @@ WHERE availability_zone.name = $availabilityZoneName.name`
`146`	`146`	`return db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error {`
`147`	`147`	`instanceData := instanceData{`
`148`	`148`	`MachineUUID: machineUUID,`
`149`		`- InstanceID: string(instanceID),`
	`149`	`+ InstanceID: instanceID.String(),`
`150`	`150`	`DisplayName: displayName,`
`151`	`151`	`}`
`152`	`152`	`if hardwareCharacteristics != nil {`
Original file line number	Diff line number	Diff line change
`@@ -152,7 +152,7 @@ func (s *Service) CheckMachines(`
`152`	`152`	`// Build the sets of provider instance IDs and model machine instance IDs.`
`153`	`153`	`providerInstanceIDsSet := make(set.Strings, len(providerInstances))`
`154`	`154`	`for _, instance := range providerInstances {`
`155`		`- providerInstanceIDsSet.Add(string(instance.Id()))`
	`155`	`+ providerInstanceIDsSet.Add(instance.Id().String())`
`156`	`156`	`}`
`157`	`157`
`158`	`158`	`instanceIDsSet, err := s.st.GetAllInstanceIDs(ctx)`