feat: Implement support for image architecture (fixes #105, thanks @K…

…night1) - switch to non-deprecated architecture-aware image lookup - use server type architecture by default - add --hetzner-image-arch for explicit setting - minor refactoring
JonasProgrammer · Apr 15, 2023 · 6915fda · 6915fda
1 parent 202612d
commit 6915fda
Show file tree

Hide file tree

Showing 3 changed files with 217 additions and 54 deletions.
diff --git a/README.md b/README.md
@@ -15,8 +15,8 @@ You can find sources and pre-compiled binaries [here](https://github.com/JonasPr
 
 ```bash
 # Download the binary (this example downloads the binary for linux amd64)
-$ wget https://github.com/JonasProgrammer/docker-machine-driver-hetzner/releases/download/3.12.2/docker-machine-driver-hetzner_3.12.2_linux_amd64.tar.gz
-$ tar -xvf docker-machine-driver-hetzner_3.12.2_linux_amd64.tar.gz
+$ wget https://github.com/JonasProgrammer/docker-machine-driver-hetzner/releases/download/3.13.0/docker-machine-driver-hetzner_3.13.0_linux_amd64.tar.gz
+$ tar -xvf docker-machine-driver-hetzner_3.13.0_linux_amd64.tar.gz
 
 # Make it executable and copy the binary in a directory accessible with your $PATH
 $ chmod +x docker-machine-driver-hetzner
@@ -91,7 +91,8 @@ $ docker-machine create \
 ## Options
 
 - `--hetzner-api-token`: **required**. Your project-specific access token for the Hetzner Cloud API.
-- `--hetzner-image`: The name of the Hetzner Cloud image to use, see [Images API](https://docs.hetzner.cloud/#resources-images-get) for how to get a list (defaults to `ubuntu-18.04`).
+- `--hetzner-image`: The name (or ID) of the Hetzner Cloud image to use, see [Images API](https://docs.hetzner.cloud/#resources-images-get) for how to get a list (defaults to `ubuntu-18.04`).
+- `--hetzner-image`: The architecture to use during image lookup, inferred from the server type if not explicitly given.
 - `--hetzner-image-id`: The id of the Hetzner cloud image (or snapshot) to use, see [Images API](https://docs.hetzner.cloud/#resources-images-get) for how to get a list (mutually excludes `--hetzner-image`).
 - `--hetzner-server-type`: The type of the Hetzner Cloud server, see [Server Types API](https://docs.hetzner.cloud/#resources-server-types-get) for how to get a list (defaults to `cx11`).
 - `--hetzner-server-location`: The location to create the server in, see [Locations API](https://docs.hetzner.cloud/#resources-locations-get) for how to get a list.
@@ -115,6 +116,15 @@ $ docker-machine create \
 - `--hetzner-primary-ipv4/6`: Sets an existing primary IP (v4 or v6 respectively) for the server, as documented in [Networking](#networking)
 - `--hetzner-wait-on-error`: Amount of seconds to wait on server creation failure (0/no wait by default)
 
+#### Image selection
+
+When `--hetzner-image-id` is passed, it will be used for lookup by ID as-is. No additional validation is performed, and it is mutually exclusive with
+other `--hetzner-image*`-flags.
+
+When `--hetzner-image` is passed, lookup will happen either by name or by ID as per Hetzner-supplied logic. The lookup mechanism will filter by image
+architecture, which is usually inferred from the server type. One may explicitly specify it using `--hetzner-image-arch` in which case the user
+supplied value will take precedence.
+
 #### Existing SSH keys
 
 When you specify the `--hetzner-existing-key-path` option, the driver will attempt to copy `(specified file name)`
@@ -136,6 +146,7 @@ was used during creation.
 |---------------------------------|-------------------------------| -------------------------- |
 | **`--hetzner-api-token`**       | `HETZNER_API_TOKEN`           |                            |
 | `--hetzner-image`               | `HETZNER_IMAGE`               | `ubuntu-18.04`             |
+| `--hetzner-image-arch`          | `HETZNER_IMAGE_ARCH`          | *(infer from server)*      |
 | `--hetzner-image-id`            | `HETZNER_IMAGE_ID`            |                            |
 | `--hetzner-server-type`         | `HETZNER_TYPE`                | `cx11`                     |
 | `--hetzner-server-location`     | `HETZNER_LOCATION`            | *(let Hetzner choose)*     |

diff --git a/driver.go b/driver.go
@@ -26,6 +26,7 @@ type Driver struct {
 	AccessToken       string
 	Image             string
 	ImageID           int
+	ImageArch         hcloud.Architecture
 	cachedImage       *hcloud.Image
 	Type              string
 	cachedType        *hcloud.ServerType
@@ -69,6 +70,7 @@ const (
 	flagAPIToken          = "hetzner-api-token"
 	flagImage             = "hetzner-image"
 	flagImageID           = "hetzner-image-id"
+	flagImageArch         = "hetzner-image-arch"
 	flagType              = "hetzner-server-type"
 	flagLocation          = "hetzner-server-location"
 	flagExKeyID           = "hetzner-existing-key-id"
@@ -108,6 +110,8 @@ const (
 	legacyFlagUserDataFromFile = "hetzner-user-data-from-file"
 	legacyFlagDisablePublic4   = "hetzner-disable-public-4"
 	legacyFlagDisablePublic6   = "hetzner-disable-public-6"
+
+	emptyImageArchitecture = hcloud.Architecture("")
 )
 
 // NewDriver initializes a new driver instance; see [drivers.Driver.NewDriver]
@@ -144,6 +148,11 @@ func (d *Driver) GetCreateFlags() []mcnflag.Flag {
 			Name:   flagImageID,
 			Usage:  "Image to use for server creation",
 		},
+		mcnflag.StringFlag{
+			EnvVar: "HETZNER_IMAGE_ARCH",
+			Name:   flagImageArch,
+			Usage:  "Image architecture for lookup to use for server creation",
+		},
 		mcnflag.StringFlag{
 			EnvVar: "HETZNER_TYPE",
 			Name:   flagType,
@@ -305,12 +314,16 @@ func (d *Driver) setConfigFromFlagsImpl(opts drivers.DriverOptions) error {
 	d.AccessToken = opts.String(flagAPIToken)
 	d.Image = opts.String(flagImage)
 	d.ImageID = opts.Int(flagImageID)
+	err := d.setImageArch(opts.String(flagImageArch))
+	if err != nil {
+		return err
+	}
 	d.Location = opts.String(flagLocation)
 	d.Type = opts.String(flagType)
 	d.KeyID = opts.Int(flagExKeyID)
 	d.IsExistingKey = d.KeyID != 0
 	d.originalKey = opts.String(flagExKeyPath)
-	err := d.setUserDataFlags(opts)
+	err = d.setUserDataFlags(opts)
 	if err != nil {
 		return err
 	}
@@ -349,12 +362,45 @@ func (d *Driver) setConfigFromFlagsImpl(opts drivers.DriverOptions) error {
 		return d.flagFailure("hetzner requires --%v to be set", flagAPIToken)
 	}
 
+	if err = d.verifyImageFlags(); err != nil {
+		return err
+	}
+
+	if err = d.verifyNetworkFlags(); err != nil {
+		return err
+	}
+
+	instrumented(d)
+
+	return nil
+}
+
+func (d *Driver) setImageArch(arch string) error {
+	switch arch {
+	case "":
+		d.ImageArch = emptyImageArchitecture
+	case string(hcloud.ArchitectureARM):
+		d.ImageArch = hcloud.ArchitectureARM
+	case string(hcloud.ArchitectureX86):
+		d.ImageArch = hcloud.ArchitectureX86
+	default:
+		return errors.Errorf("unknown architecture %v", arch)
+	}
+	return nil
+}
+
+func (d *Driver) verifyImageFlags() error {
 	if d.ImageID != 0 && d.Image != "" && d.Image != defaultImage /* support legacy behaviour */ {
 		return d.flagFailure("--%v and --%v are mutually exclusive", flagImage, flagImageID)
+	} else if d.ImageID != 0 && d.ImageArch != "" {
+		return d.flagFailure("--%v and --%v are mutually exclusive", flagImageArch, flagImageID)
 	} else if d.ImageID == 0 && d.Image == "" {
 		d.Image = defaultImage
 	}
+	return nil
+}
 
+func (d *Driver) verifyNetworkFlags() error {
 	if d.DisablePublic4 && d.DisablePublic6 && !d.UsePrivateNetwork {
 		return d.flagFailure("--%v must be used if public networking is disabled (hint: implicitly set by --%v)",
 			flagUsePrivateNetwork, flagDisablePublic)
@@ -367,9 +413,6 @@ func (d *Driver) setConfigFromFlagsImpl(opts drivers.DriverOptions) error {
 	if d.DisablePublic6 && d.PrimaryIPv6 != "" {
 		return d.flagFailure("--%v and --%v are mutually exclusive", flagPrimary6, flagDisablePublic6)
 	}
-
-	instrumented(d)
-
 	return nil
 }
 
@@ -437,35 +480,14 @@ func (d *Driver) setLabelsFromFlags(opts drivers.DriverOptions) error {
 
 // PreCreateCheck validates the Driver data is in a valid state for creation; see [drivers.Driver.PreCreateCheck]
 func (d *Driver) PreCreateCheck() error {
-	if d.IsExistingKey {
-		if d.originalKey == "" {
-			return d.flagFailure("specifying an existing key ID requires the existing key path to be set as well")
-		}
-
-		key, err := d.getKey()
-		if err != nil {
-			return errors.Wrap(err, "could not get key")
-		}
-
-		buf, err := os.ReadFile(d.originalKey + ".pub")
-		if err != nil {
-			return errors.Wrap(err, "could not read public key")
-		}
-
-		// Will also parse `ssh-rsa w309jwf0e39jf asdf` public keys
-		pubk, _, _, _, err := ssh.ParseAuthorizedKey(buf)
-		if err != nil {
-			return errors.Wrap(err, "could not parse authorized key")
-		}
-
-		if key.Fingerprint != ssh.FingerprintLegacyMD5(pubk) &&
-			key.Fingerprint != ssh.FingerprintSHA256(pubk) {
-			return errors.Errorf("remote key %d does not match local key %s", d.KeyID, d.originalKey)
-		}
+	if err := d.setupExistingKey(); err != nil {
+		return err
 	}
 
-	if _, err := d.getType(); err != nil {
+	if serverType, err := d.getType(); err != nil {
 		return errors.Wrap(err, "could not get type")
+	} else if d.ImageArch != "" && serverType.Architecture != d.ImageArch {
+		log.Warnf("supplied architecture %v differs from server architecture %v", d.ImageArch, serverType.Architecture)
 	}
 
 	if _, err := d.getImage(); err != nil {
@@ -495,6 +517,39 @@ func (d *Driver) PreCreateCheck() error {
 	return nil
 }
 
+func (d *Driver) setupExistingKey() error {
+	if !d.IsExistingKey {
+		return nil
+	}
+
+	if d.originalKey == "" {
+		return d.flagFailure("specifying an existing key ID requires the existing key path to be set as well")
+	}
+
+	key, err := d.getKey()
+	if err != nil {
+		return errors.Wrap(err, "could not get key")
+	}
+
+	buf, err := os.ReadFile(d.originalKey + ".pub")
+	if err != nil {
+		return errors.Wrap(err, "could not read public key")
+	}
+
+	// Will also parse `ssh-rsa w309jwf0e39jf asdf` public keys
+	pubk, _, _, _, err := ssh.ParseAuthorizedKey(buf)
+	if err != nil {
+		return errors.Wrap(err, "could not parse authorized key")
+	}
+
+	if key.Fingerprint != ssh.FingerprintLegacyMD5(pubk) &&
+		key.Fingerprint != ssh.FingerprintSHA256(pubk) {
+		return errors.Errorf("remote key %d does not match local key %s", d.KeyID, d.originalKey)
+	}
+
+	return nil
+}
+
 // Create actually creates the hetzner-cloud server; see [drivers.Driver.Create]
 func (d *Driver) Create() error {
 	err := d.prepareLocalKey()
@@ -872,26 +927,8 @@ func (d *Driver) GetState() (state.State, error) {
 
 // Remove deletes the hetzner server and additional resources created during creation; see [drivers.Driver.Remove]
 func (d *Driver) Remove() error {
-	if d.ServerID != 0 {
-		srv, err := d.getServerHandle()
-		if err != nil {
-			return errors.Wrap(err, "could not get server handle")
-		}
-
-		if srv == nil {
-			log.Infof(" -> Server does not exist anymore")
-		} else {
-			log.Infof(" -> Destroying server %s[%d] in...", srv.Name, srv.ID)
-
-			if _, err := d.getClient().Server.Delete(context.Background(), srv); err != nil {
-				return errors.Wrap(err, "could not delete server")
-			}
-
-			// failure to remove a placement group is not a hard error
-			if softErr := d.removeEmptyServerPlacementGroup(srv); softErr != nil {
-				log.Error(softErr)
-			}
-		}
+	if err := d.destroyServer(); err != nil {
+		return err
 	}
 
 	// failure to remove a key is not ha hard error
@@ -931,6 +968,40 @@ func (d *Driver) Remove() error {
 	return nil
 }
 
+func (d *Driver) destroyServer() error {
+	if d.ServerID == 0 {
+		return nil
+	}
+
+	srv, err := d.getServerHandle()
+	if err != nil {
+		return errors.Wrap(err, "could not get server handle")
+	}
+
+	if srv == nil {
+		log.Infof(" -> Server does not exist anymore")
+	} else {
+		log.Infof(" -> Destroying server %s[%d] in...", srv.Name, srv.ID)
+
+		res, _, err := d.getClient().Server.DeleteWithResult(context.Background(), srv)
+		if err != nil {
+			return errors.Wrap(err, "could not delete server")
+		}
+
+		// failure to remove a placement group is not a hard error
+		if softErr := d.removeEmptyServerPlacementGroup(srv); softErr != nil {
+			log.Error(softErr)
+		}
+
+		// wait for the server to actually be deleted
+		if err = d.waitForAction(res.Action); err != nil {
+			return errors.Wrap(err, "could not wait for deletion")
+		}
+	}
+
+	return nil
+}
+
 // Restart instructs the hetzner cloud server to reboot; see [drivers.Driver.Restart]
 func (d *Driver) Restart() error {
 	srv, err := d.getServerHandle()
@@ -1071,7 +1142,12 @@ func (d *Driver) getImage() (*hcloud.Image, error) {
 			return image, errors.Wrap(err, fmt.Sprintf("could not get image by id %v", d.ImageID))
 		}
 	} else {
-		image, _, err = d.getClient().Image.GetByName(context.Background(), d.Image)
+		arch, err := d.getImageArchitectureForLookup()
+		if err != nil {
+			return nil, errors.Wrap(err, "could not determine image architecture")
+		}
+
+		image, _, err = d.getClient().Image.GetByNameAndArchitecture(context.Background(), d.Image, arch)
 		if err != nil {
 			return image, errors.Wrap(err, fmt.Sprintf("could not get image by name %v", d.Image))
 		}
@@ -1081,6 +1157,19 @@ func (d *Driver) getImage() (*hcloud.Image, error) {
 	return instrumented(image), nil
 }
 
+func (d *Driver) getImageArchitectureForLookup() (hcloud.Architecture, error) {
+	if d.ImageArch != emptyImageArchitecture {
+		return d.ImageArch, nil
+	}
+
+	serverType, err := d.getType()
+	if err != nil {
+		return "", err
+	}
+
+	return serverType.Architecture, nil
+}
+
 func (d *Driver) getKey() (*hcloud.SSHKey, error) {
 	if d.cachedKey != nil {
 		return d.cachedKey, nil