nanovms / ops

ops - build and run nanos unikernels
https://ops.city
MIT License
1.27k stars 132 forks source link

gcp(tags): backward compatible extended usage #1542

Closed rinor closed 10 months ago

rinor commented 10 months ago

I need a bit more control over what goes where. The existing behavior is kept the same, unless one uses the new specific definitions.

eyberg commented 10 months ago

tags are heavily used by quite a few people/orgs so I do like the backwards compatibility of this; one change that i might suggest if you want to break it up into varying types - create a new tag struct and stick an enum in it to define the type with the default choice being the old style (eg: used for everything)

rinor commented 10 months ago

tags are heavily used by quite a few people/orgs so I do like the backwards compatibility of this; one change that i might suggest if you want to break it up into varying types - create a new tag struct and stick an enum in it to define the type with the default choice being the old style (eg: used for everything)

what if we add an extended attribute to tags, containing the new specific attributes, that if present and set, will activate and use new functionality, i.e:

  1. Existing behavior, remains unchanged, if configured as always
{
  "CloudConfig": {
    "Tags": [
      {
        "key": "instance-and-image",
        "value": "instance-image"
      }
    ]
  }
}
  1. Equivalent behavior/result if using the new config
{
  "CloudConfig": {
    "Tags": [
      {
        "key": "instance-and-image",
        "value": "instance-image",
        "attribute": {
          "image_label": true,
          "instance_label": true
        }
      }
    ]
  }
}
  1. If preferred and desired, feedback needed here, we could even treat an empty attribute list as not set, so this config would have the same behavior/result as (1. and 2.)
{
  "CloudConfig": {
    "Tags": [
      {
        "key": "instance-and-image",
        "value": "instance-image",
        "attribute": {}
      }
    ]
  }
}
  1. And this would be different from (3.), since as soon as you add at least one field inside attribute (true or false it doesn't matter), you opt-in for the new explicit config:
    {
    "CloudConfig": {
    "Tags": [
      {
        "key": "this-goes-nowhere",
        "value": "non-existent",
        "attribute": {
          "image_label": false,
          "instance_label": false,
          "instance_network": false,
          "instance_metadata": false
        }
      }
    ]
    }
    }

In the meantime, I will update the code so we can check what it could look like

eyberg commented 10 months ago

yeh - makes sense - i'm good w/these changes - is this ready to merge?

rinor commented 10 months ago

yeh - makes sense - i'm good w/these changes - is this ready to merge?

yes, will push also the updated ops docs a bit later