Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cloud metadata #748

Merged
merged 18 commits into from
Sep 24, 2021
Merged

Conversation

dacbd
Copy link
Contributor

@dacbd dacbd commented Sep 24, 2021

Closes #430
Supercedes or Blocked by: #747
Requires upstream: iterative/terraform-provider-iterative#185

Manual testing:

./bin/cml.js runner \
    --single \
    --log=debug \
    --idle-timeout=1800 \
    --token=xxx \
    --cloud=gcp \
    --cloud-region=us-west \
    --cloud-type=m \
    --cloud-metadata="mykey=myval" \
    --cloud-gpu=nogpu \
    --repo=xxx \
    --driver=github \
    --cloud-hdd-size=10

Terraform template:

terraform {
  required_providers {
    iterative = {
      source = "iterative/iterative"
    }
  }
}

provider "iterative" {}


resource "iterative_cml_runner" "runner" {
  repo = "xxx"
  token = "xxx"
  driver = "github"
  labels = "cml"
  idle_timeout = 1800
  name = "cml-40elk3xchx"
  single = "true"
  cloud = "gcp"
  region = "us-west"
  instance_type = "m"
  
  instance_hdd_size = 10
  
  
  spot_price = -1
  
  
  metadata = {
    mykey = "myval"
  }
}

Test:

./bin/cml.js runner \
    --single \
    --log=debug \
    --idle-timeout=1800 \
    --token=xxx \
    --cloud=gcp \
    --cloud-region=us-west \
    --cloud-type=m \
    --cloud-metadata="mykey=myval" \
    --cloud-metadata="mykey2=myval2" \
    --cloud-metadata="mykey3=myval3" \
    --cloud-gpu=nogpu \
    --repo=xxx \
    --driver=github \
    --cloud-hdd-size=10

Terraform template:

terraform {
  required_providers {
    iterative = {
      source = "iterative/iterative"
    }
  }
}

provider "iterative" {}


resource "iterative_cml_runner" "runner" {
  repo = "xxx"
  token = "xxx"
  driver = "github"
  labels = "cml"
  idle_timeout = 1800
  name = "cml-40elk3xchx"
  single = "true"
  cloud = "gcp"
  region = "us-west"
  instance_type = "m"
  
  instance_hdd_size = 10
  
  
  spot_price = -1
  
  
  metadata = {
    mykey = "myval"
    mykey2 = "myval2"
    mykey3 = "myval3"
  }
}

@dacbd dacbd temporarily deployed to external September 24, 2021 18:28 Inactive
@dacbd dacbd temporarily deployed to external September 24, 2021 18:53 Inactive
@dacbd
Copy link
Contributor Author

dacbd commented Sep 24, 2021

@0x2b3bfa0 or @DavidGOrtega are you able to add a contrib branch and change the PR target while it is still marked as a draft?

@dacbd dacbd temporarily deployed to external September 24, 2021 19:12 Inactive
@dacbd dacbd temporarily deployed to external September 24, 2021 19:45 Inactive
@dacbd
Copy link
Contributor Author

dacbd commented Sep 24, 2021

Updated based on: #430 (comment)

@dacbd dacbd marked this pull request as ready for review September 24, 2021 19:59
Additionally,
* Handle falsy values as empty strings
* Simplify template code
@0x2b3bfa0
Copy link
Member

are you able to add a contrib branch and change the PR target while it is still marked as a draft?

Yes, we can. Would you like to merge it first to a contrib branch on our repository?

@0x2b3bfa0
Copy link
Member

0x2b3bfa0 commented Sep 24, 2021

@danielbarnes, counter–attacking with some suggestions and updated tests on dacbd#1. Feel free to review and merge that into your pull request.

@dacbd dacbd temporarily deployed to external September 24, 2021 20:47 Inactive
@0x2b3bfa0 0x2b3bfa0 self-requested a review September 24, 2021 20:56
@0x2b3bfa0 0x2b3bfa0 added the enhancement New feature or request label Sep 24, 2021
@dacbd
Copy link
Contributor Author

dacbd commented Sep 24, 2021

are you able to add a contrib branch and change the PR target while it is still marked as a draft?

Yes, we can. Would you like to merge it first to a contrib branch on our repository?

I'm fine with 'owning' this patch should any changes be required but I'm still knocking off ~6yrs of nodejs rust and would prefer it lives on a branch here since upstream changes need to be published before it can really be tested.

and I have a bad habit of blasting my own repos and re-forking things 😨

Copy link
Member

@0x2b3bfa0 0x2b3bfa0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@0x2b3bfa0 0x2b3bfa0 changed the base branch from master to daniel-barnes-cloud-metadata September 24, 2021 21:01
@0x2b3bfa0 0x2b3bfa0 merged commit 8ebaf5f into iterative:daniel-barnes-cloud-metadata Sep 24, 2021
@0x2b3bfa0
Copy link
Member

Merged into a separate branch. We'll release it after the Terraform provider.

@0x2b3bfa0 0x2b3bfa0 mentioned this pull request Sep 24, 2021
DavidGOrtega added a commit that referenced this pull request Oct 15, 2021
* Convert yargs builder function calls to plain objects

Leave environment variable management to yargs

* Remove dead code

* Manage all the environment variables through yargs (#739)

* Fix comment blunder

* replace null

* tf template check

* add null option

* clean up

* use undefined

* rm redundant debug logging

* update from draft example

* clean up

* Update tests

Additionally,
* Handle falsy values as empty strings
* Simplify template code

Co-authored-by: Helio Machado <[email protected]>

Co-authored-by: Daniel Barnes <[email protected]>
Co-authored-by: davidgortega <[email protected]>
0x2b3bfa0 added a commit that referenced this pull request Oct 15, 2021
* replace null

* tf template check

* add null option

* clean up

* use undefined

* rm redundant debug logging

* update from draft example

* clean up

* Update tests

Additionally,
* Handle falsy values as empty strings
* Simplify template code
0x2b3bfa0 added a commit that referenced this pull request Oct 15, 2021
* replace null

* tf template check

* add null option

* clean up

* use undefined

* rm redundant debug logging

* update from draft example

* clean up

* Update tests

Additionally,
* Handle falsy values as empty strings
* Simplify template code

Co-authored-by: Helio Machado <[email protected]>
@DavidGOrtega DavidGOrtega mentioned this pull request Oct 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tagging of instances on AWS
3 participants