refactor: incorporate changes from #73

Author: JonasProgrammer

README.md

@@ -128,22 +128,22 @@ was used during creation.
 
 | CLI option                          | Environment variable              | Default                    |
 | ----------------------------------- | --------------------------------- | -------------------------- |
-| **`--hetzner-api-token`**           | `HETZNER_API_TOKEN`               | -                          |
+| **`--hetzner-api-token`**           | `HETZNER_API_TOKEN`               |                            |
 | `--hetzner-image`                   | `HETZNER_IMAGE`                   | `ubuntu-18.04`             |
-| `--hetzner-image-id`                | `HETZNER_IMAGE_ID`                | -                          |
+| `--hetzner-image-id`                | `HETZNER_IMAGE_ID`                |                            |
 | `--hetzner-server-type`             | `HETZNER_TYPE`                    | `cx11`                     |
-| `--hetzner-server-location`         | `HETZNER_LOCATION`                | - *(let Hetzner choose)*   |
-| `--hetzner-existing-key-path`       | `HETZNER_EXISTING_KEY_PATH`       | - *(generate new keypair)* |
+| `--hetzner-server-location`         | `HETZNER_LOCATION`                | *(let Hetzner choose)*     |
+| `--hetzner-existing-key-path`       | `HETZNER_EXISTING_KEY_PATH`       | *(generate new keypair)*   |
 | `--hetzner-existing-key-id`         | `HETZNER_EXISTING_KEY_ID`         | 0 *(upload new key)*       |
-| `--hetzner-additional-key`          | `HETZNER_ADDITIONAL_KEYS`         | -                          |
-| `--hetzner-user-data`               | `HETZNER_USER_DATA`               | -                          |
-| `--hetzner-networks`                | `HETZNER_NETWORKS`                | -                          |
-| `--hetzner-firewalls`               | `HETZNER_FIREWALLS`               | -                          |
-| `--hetzner-volumes`                 | `HETZNER_VOLUMES`                 | -                          |
+| `--hetzner-additional-key`          | `HETZNER_ADDITIONAL_KEYS`         |                            |
+| `--hetzner-user-data`               | `HETZNER_USER_DATA`               |                            |
+| `--hetzner-networks`                | `HETZNER_NETWORKS`                |                            |
+| `--hetzner-firewalls`               | `HETZNER_FIREWALLS`               |                            |
+| `--hetzner-volumes`                 | `HETZNER_VOLUMES`                 |                            |
 | `--hetzner-use-private-network`     | `HETZNER_USE_PRIVATE_NETWORK`     | false                      |
 | `--hetzner-server-label`            | (inoperative)                     | `[]`                       |
 | `--hetzner-key-label`               | (inoperative)                     | `[]`                       |
-| `--hetzner-placement-group`         | `HETZNER_PLACEMENT_GROUP`         | -                          |
+| `--hetzner-placement-group`         | `HETZNER_PLACEMENT_GROUP`         |                            |
 | `--hetzner-auto-spread`             | `HETZNER_AUTO_SPREAD`             | false                      |
 
 ## Building from source

driver.go

@@ -657,29 +657,30 @@ func (d *Driver) Remove() error {
 				return errors.Wrap(err, "could not delete server")
 			}
 
-			err = d.removeEmptyServerPlacementGroup(srv)
-			if err != nil {
-				log.Error(err) // not a hard failure
+			// failure to remove a placement group is not a hard error
+			if softErr := d.removeEmptyServerPlacementGroup(srv); softErr != nil {
+				log.Error(softErr)
 			}
 		}
 	}
 
-	// Failing to remove these is just a soft error
+	// failure to remove a key is not ha hard error
 	for i, id := range d.AdditionalKeyIDs {
 		log.Infof(" -> Destroying additional key #%d (%d)", i, id)
-		key, _, err := d.getClient().SSHKey.GetByID(context.Background(), id)
-		if err != nil {
-			log.Warnf(" ->  -> could not retrieve key %v", err)
+		key, _, softErr := d.getClient().SSHKey.GetByID(context.Background(), id)
+		if softErr != nil {
+			log.Warnf(" ->  -> could not retrieve key %v", softErr)
 		} else if key == nil {
 			log.Warnf(" ->  -> %d no longer exists", id)
 		}
 
-		_, err = d.getClient().SSHKey.Delete(context.Background(), key)
-		if err != nil {
-			log.Warnf(" ->  -> could not remove key: %v", err)
+		_, softErr = d.getClient().SSHKey.Delete(context.Background(), key)
+		if softErr != nil {
+			log.Warnf(" ->  -> could not remove key: %v", softErr)
 		}
 	}
 
+	// failure to remove a server-specific key is a hard error
 	if !d.IsExistingKey && d.KeyID != 0 {
 		key, err := d.getKey()
 		if err != nil {
@@ -953,16 +954,16 @@ func (d *Driver) makePlacementGroup(name string, labels map[string]string) (*hcl
 		d.dangling = append(d.dangling, func() {
 			_, err := d.getClient().PlacementGroup.Delete(context.Background(), grp.PlacementGroup)
 			if err != nil {
-				log.Error(fmt.Errorf("could not delete placement group: %w", err))
+				log.Errorf("could not delete placement group: %v", err)
 			}
 		})
 	}
 
 	if err != nil {
-		err = fmt.Errorf("could not create placement group: %w", err)
+		return nil, fmt.Errorf("could not create placement group: %w", err)
 	}
 
-	return grp.PlacementGroup, err
+	return grp.PlacementGroup, nil
 }
 
 func (d *Driver) getPlacementGroup() (*hcloud.PlacementGroup, error) {
@@ -1006,9 +1007,9 @@ func (d *Driver) removeEmptyServerPlacementGroup(srv *hcloud.Server) error {
 	if auto, exists := pg.Labels[d.labelName(labelAutoCreated)]; exists && auto == "true" {
 		_, err := d.getClient().PlacementGroup.Delete(context.Background(), pg)
 		if err != nil {
-			err = fmt.Errorf("could not remove placement group: %w", err)
+			return fmt.Errorf("could not remove placement group: %w", err)
 		}
-		return err
+		return nil
 	} else {
 		log.Debugf("group not auto-created, ignoring: %v", pg)
 		return nil