Allow for configuring a specific CML version #255

dacbd · 2021-11-22T21:48:17Z

I haven't played with the task branch yet so, with those changes in mind how relevant would the following be?

I want to be able to target a specific cml version here:

terraform-provider-iterative/iterative/resource_runner.go

Line 288 in 8121c66

sudo npm config set user 0 && sudo npm install --global @dvcorg/cml

similar to this

adding an optional cml_version / defaulting to @dvcorg/cml

terraform-provider-iterative/iterative/resource_runner.go

Lines 409 to 433 in 8121c66

    
           data := make(map[string]interface{}) 
        
           data["token"] = d.Get("token").(string) 
        
           data["repo"] = d.Get("repo").(string) 
        
           data["driver"] = d.Get("driver").(string) 
        
           data["labels"] = d.Get("labels").(string) 
        
           data["idle_timeout"] = strconv.Itoa(d.Get("idle_timeout").(int)) 
        
           data["name"] = d.Get("name").(string) 
        
           data["cloud"] = d.Get("cloud").(string) 
        
           data["startup_script"] = d.Get("startup_script").(string) 
        
           data["tf_resource"] = base64.StdEncoding.EncodeToString(jsonResource) 
        
           data["instance_gpu"] = d.Get("instance_gpu").(string) 
        
           data["single"] = d.Get("single").(bool) 
        
           data["AWS_SECRET_ACCESS_KEY"] = os.Getenv("AWS_SECRET_ACCESS_KEY") 
        
           data["AWS_ACCESS_KEY_ID"] = os.Getenv("AWS_ACCESS_KEY_ID") 
        
           data["AWS_SESSION_TOKEN"] = os.Getenv("AWS_SESSION_TOKEN") 
        
           data["AZURE_CLIENT_ID"] = os.Getenv("AZURE_CLIENT_ID") 
        
           data["AZURE_CLIENT_SECRET"] = os.Getenv("AZURE_CLIENT_SECRET") 
        
           data["AZURE_SUBSCRIPTION_ID"] = os.Getenv("AZURE_SUBSCRIPTION_ID") 
        
           data["AZURE_TENANT_ID"] = os.Getenv("AZURE_TENANT_ID") 
        
           data["GOOGLE_APPLICATION_CREDENTIALS_DATA"] = os.Getenv("GOOGLE_APPLICATION_CREDENTIALS_DATA") 
        
           data["KUBERNETES_CONFIGURATION"] = os.Getenv("KUBERNETES_CONFIGURATION") 
        
           data["container"] = isContainerAvailable(d.Get("cloud").(string)) 
        
           data["setup"] = strings.Replace(string(setup[:]), "#/bin/sh", "", 1) 
        
           return renderScript(data)

Thoughts @DavidGOrtega @0x2b3bfa0 ?

casperdcl · 2021-11-23T14:46:04Z

I feel like this sort of thing (deps & versions of deps) should be part of the user's startup script or task instead... Should TPI really be particularly CML-aware?

0x2b3bfa0 · 2021-11-23T15:20:31Z

This suggestion is not applicable to the iterative_task resource, because it isn't aware of CML in any way, and this would be handled on the users' startup scripts as @casperdcl points out.

Although, the legacy iterative_cml_runner resource is specific for CML runners and, as such, it would make sense to allow pinning any given version. 🤷🏼‍♂️

0x2b3bfa0 · 2021-11-23T15:23:10Z

Still, we¹ chose to keep CML under the same minor version and avoid introducing breaking changes, so in theory, there shouldn't be any need of pinning it.

actually they, in the year 1 before @0x2b3bfa0 ↩

dacbd · 2021-11-23T15:53:14Z

I'll play with task today, and I have a few ideas how I want to use it.

DavidGOrtega · 2021-12-15T22:13:45Z

@dacbd I think that might be very interesting. CML should be backwards compatible, at least thats the philosophy but since we allow to pin in setup-cml it should make sense here.

Just only one possible issue:
I thinks is a good idea replace CML npm package and node deps with the standalone CLI after #842

Any thoughts @dacbd @iterative/cml ?

dacbd · 2021-12-16T00:07:48Z

I think it depends on how it's executed. I haven't really experienced any issues with the node deps with CML but I am well aware of the issues the ecosystem can have, plus it makes installing/portability simpler which I feel is your guys targeted goal.

I see/have no issues against a standalone CLI. I personally feel that tools like these should have strong control/influence on their dependencies. I did a lot of work with the hapi.js framework a long time ago and their control over their dependencies offered a stable product than others.

You never know what bonehead doesn't have 2FA on their npm account and now your dependency has a crypto-miner or a package.json build script dialing home, granted these things get caught pretty fast these days, and realistically they normally have a low impact but the damage to the "name" is done anyway.

0x2b3bfa0 · 2022-01-21T20:13:56Z

Since v0.10.0 (iterative/cml#825), CML can be installed as a single, standalone binary file.

0x2b3bfa0 · 2022-01-21T20:15:11Z

dacbd · 2022-01-21T20:23:07Z

I'll take a look, plan to incorporate something this weekend that you can take a look at.

0x2b3bfa0 · 2022-02-22T16:57:04Z

I'd even remove the “classical” NPM support and include only the required logic for downloading binary releases. Apart from that, looks good to me! 🎉 @iterative/cml?

dacbd · 2022-02-22T17:32:25Z

I'd even remove the “classical” NPM support ...

This part of my covert 🔐 effort to make testing custom changes(in cml/tpi) easier (npm provides a convenient method for "installing" directly from a git ref), but that is secondary to actual function. 😄

0x2b3bfa0 · 2022-02-22T20:49:26Z

Tip: cd iterative && go test -update to fix snapshot tests

dacbd · 2022-02-22T21:28:54Z

I will make a couple house cleaning changes tonight and mark it ready. (saw at least one empty line that was superfluous)

dacbd · 2022-02-23T05:30:30Z

Boy, can I say my brain wasn't working tonight for some reason...

After brute-forcing myself back to functional with some hacky e2e testing, I formed a couple questions:

Should this be updated further?

terraform-provider-iterative/iterative/resource_runner_test.go

Lines 35 to 70 in 35ce161

    
           func TestProvisionerCode(t *testing.T) { 
        
           	g := goldie.New(t, goldie.WithDiffEngine(goldie.ColoredDiff)) 
        
           	for _, cloud := range []string{"aws", "azure", "gcp", "kubernetes", "invalid"} { 
        
           		t.Run("Provisioner code for "+cloud+" should pass golden test", func(t *testing.T) { 
        
           			val, err := renderProvisionerCode(t, cloud) 
        
           			assert.Nil(t, err) 
        
           			g.Assert(t, "script_template_cloud_"+cloud, []byte(val)) 
        
           		}) 
        
           	} 
        
           } 
        
           func renderProvisionerCode(t *testing.T, cloud string) (string, error) { 
        
           	// Note for future tests: this code modifies process environment variables. 
        
           	for key, value := range generateEnvironmentTestData(t) { 
        
           		os.Setenv(key, value) 
        
           	} 
        
           	return provisionerCode(generateSchemaTestData(cloud, t)) 
        
           } 
        
           func generateSchemaTestData(cloud string, t *testing.T) *schema.ResourceData { 
        
           	return schema.TestResourceDataRaw(t, resourceRunner().Schema, map[string]interface{}{ 
        
           		"cloud":                 cloud, 
        
           		"region":                "9 value with \"quotes\" and spaces", 
        
           		"name":                  "10 value with \"quotes\" and spaces", 
        
           		"single":                true, 
        
           		"idle_timeout":          11, 
        
           		"instance_hdd_size":     12, 
        
           		"token":                 "13 value with \"quotes\" and spaces", 
        
           		"repo":                  "14 value with \"quotes\" and spaces", 
        
           		"driver":                "15 value with \"quotes\" and spaces", 
        
           		"labels":                "16 value with \"quotes\" and spaces", 
        
           		"instance_gpu":          "17 value with \"quotes\" and spaces", 
        
           		"runner_startup_script": "echo \"custom startup script\"", 
        
           	}) 
        
           }

Should Installing CML be part of .setup https://github.com/iterative/terraform-provider-iterative/blob/master/environment/setup.sh ?

Should that script actually log into that file? /var/log/cml_stack.log

I'm fairly confident that I have unbroken all the things I broke.
hacky e2e test:

git clone [email protected]:dacbd/terraform-provider-iterative.git
pushd terraform-provider-iterative
git checkout target-cml-version
make build
make install
popd
git clone [email protected]:dacbd/cml.git
pushd cml
npm install
sed -i 's/source = "iterative\/iterative"/source = "github.com\/iterative\/iterative"/g' src/terraform.js
sed -i 's/return template;/console.log(template);return template;/g' src/terraform.js
bin/cml.js runner \
    --single \
    --token=***\
    --cloud=aws \
    --cloud-region=us-west-2 \
    --cloud-type=t3.micro \
    --cloud-startup-script=ZWNobyAiZWNkc2Etc2hhMi1uaXN0cDM4NCBBQUFBRTJWalpITmhMWE5vWVRJdGJtbHpkSEF6T0RRQUFBQUlibWx6ZEhBek9EUUFBQUJoQkRZZDNzc2E2TDE1alFDNWJja0oydmlXbEExdEJ5Z3hlV295M3MwUzE0WkhNeFVNZnA3dTJ5cWZpY3BITzViK3BqZ2c3THorODBJYncxNTd3YVRaUE0reGJGMi9LR3FTN2FZVjBML1I4VmJXalZFcHp4WkVlb3hTQ3dGQTF0SFdVZz09IGNhcmRubzowMDA2MTE4OTU1NzUiID4+IC9ob21lL3VidW50dS8uc3NoL2F1dGhvcml6ZWRfa2V5cwo=
popd
rm -rf cml
rm -rf terraform-provider-iterative

I tested it with aws/gcp (gcp lets you view the startup script the instance was made with which was nice)
I did not test all of the paths basically just cml_version = "" ie default/latest since there isn't a method in cml runner to set it.

no longer relevant.

~~GCP gave me:~~

$ sudo systemctl status cml.service 
● cml.service
     Loaded: loaded (/etc/systemd/system/cml.service; enabled; vendor preset: enabled)
     Active: failed (Result: exit-code) since Wed 2022-02-23 05:28:52 UTC; 4min 47s ago
    Process: 17550 ExecStart=/usr/bin/cml.sh (code=exited, status=126)
   Main PID: 17550 (code=exited, status=126)

Feb 23 05:28:52 cml-w7idw4qbhm systemd[1]: Started cml.service.
Feb 23 05:28:52 cml-w7idw4qbhm cml.sh[17550]: /usr/bin/cml.sh: 17: exec: cml: Permission denied
Feb 23 05:28:52 cml-w7idw4qbhm systemd[1]: cml.service: Main process exited, code=exited, status=126/n/a
Feb 23 05:28:52 cml-w7idw4qbhm systemd[1]: cml.service: Failed with result 'exit-code'.

~~they automagically moved the cml binary~~

~~- [ ] solve later~~

lol wut google error?

I saw the instance appear with a spinning status in the GCP dashboard but they spit this error out...

https://gist.github.com/dacbd/41b9821c200dd38709a3f671b830ac49

- squashed

dacbd · 2022-02-27T04:45:12Z

Made a cml-version branch to test additional values

cml_version = ""
cml_version = "0.11.1"
cml_version = "v0.11.1"
cml_version = "git://github.com/dacbd/cml.git#cml-version"
cml_version = "0.10.0"
cml_version = "0.9.0"
cml_version = "v0.8.1"

iterative/utils/helpers.go

dacbd mentioned this pull request Dec 1, 2021

cml runner early shutdown from idle-timeout with active job iterative/cml#808

Closed

restyled-io bot mentioned this pull request Dec 20, 2021

Restyle Allow for configuring a specific CML version #337

Closed

dacbd mentioned this pull request Jan 4, 2022

Fix runner termination in AWS with non cloud regions #344

Merged

dacbd mentioned this pull request Feb 18, 2022

specify TPI version iterative/cml#885

Merged

0x2b3bfa0 approved these changes Feb 22, 2022

View reviewed changes

Lets just say I brute force every error I hit 😎

7dd4915

- squashed

dacbd force-pushed the target-cml-version branch from d058a74 to 7dd4915 Compare February 27, 2022 04:44

got test -update

afb8e58

dacbd marked this pull request as ready for review February 27, 2022 04:47

dacbd requested a review from 0x2b3bfa0 February 27, 2022 04:49

0x2b3bfa0 approved these changes Feb 27, 2022

View reviewed changes

This comment was marked as outdated.

Sign in to view

0x2b3bfa0 reviewed Feb 28, 2022

View reviewed changes

iterative/utils/helpers.go Outdated Show resolved Hide resolved

Simplify semver handling

bb9460b

0x2b3bfa0 reviewed Feb 28, 2022

View reviewed changes

iterative/utils/helpers.go Outdated Show resolved Hide resolved

Fix previous commit 🙈

fcab4f4

0x2b3bfa0 approved these changes Feb 28, 2022

View reviewed changes

0x2b3bfa0 merged commit 67a0c8c into iterative:master Feb 28, 2022

0x2b3bfa0 mentioned this pull request Mar 23, 2022

cml <sub-cmd> on cml runner instance fails iterative/cml#920

Closed

	data := make(map[string]interface{})
	data["token"] = d.Get("token").(string)
	data["repo"] = d.Get("repo").(string)
	data["driver"] = d.Get("driver").(string)
	data["labels"] = d.Get("labels").(string)
	data["idle_timeout"] = strconv.Itoa(d.Get("idle_timeout").(int))
	data["name"] = d.Get("name").(string)
	data["cloud"] = d.Get("cloud").(string)
	data["startup_script"] = d.Get("startup_script").(string)
	data["tf_resource"] = base64.StdEncoding.EncodeToString(jsonResource)
	data["instance_gpu"] = d.Get("instance_gpu").(string)
	data["single"] = d.Get("single").(bool)
	data["AWS_SECRET_ACCESS_KEY"] = os.Getenv("AWS_SECRET_ACCESS_KEY")
	data["AWS_ACCESS_KEY_ID"] = os.Getenv("AWS_ACCESS_KEY_ID")
	data["AWS_SESSION_TOKEN"] = os.Getenv("AWS_SESSION_TOKEN")
	data["AZURE_CLIENT_ID"] = os.Getenv("AZURE_CLIENT_ID")
	data["AZURE_CLIENT_SECRET"] = os.Getenv("AZURE_CLIENT_SECRET")
	data["AZURE_SUBSCRIPTION_ID"] = os.Getenv("AZURE_SUBSCRIPTION_ID")
	data["AZURE_TENANT_ID"] = os.Getenv("AZURE_TENANT_ID")
	data["GOOGLE_APPLICATION_CREDENTIALS_DATA"] = os.Getenv("GOOGLE_APPLICATION_CREDENTIALS_DATA")
	data["KUBERNETES_CONFIGURATION"] = os.Getenv("KUBERNETES_CONFIGURATION")
	data["container"] = isContainerAvailable(d.Get("cloud").(string))
	data["setup"] = strings.Replace(string(setup[:]), "#/bin/sh", "", 1)

	return renderScript(data)

Allow for configuring a specific CML version #255

Allow for configuring a specific CML version #255

Uh oh!

Conversation

dacbd commented Nov 22, 2021

Uh oh!

casperdcl commented Nov 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0x2b3bfa0 commented Nov 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0x2b3bfa0 commented Nov 23, 2021

Footnotes

Uh oh!

dacbd commented Nov 23, 2021

Uh oh!

DavidGOrtega commented Dec 15, 2021

Uh oh!

dacbd commented Dec 16, 2021

Uh oh!

0x2b3bfa0 commented Jan 21, 2022

Uh oh!

0x2b3bfa0 commented Jan 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dacbd commented Jan 21, 2022

Uh oh!

0x2b3bfa0 commented Feb 22, 2022

Uh oh!

dacbd commented Feb 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0x2b3bfa0 commented Feb 22, 2022

Uh oh!

dacbd commented Feb 22, 2022

Uh oh!

dacbd commented Feb 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dacbd commented Feb 27, 2022

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

casperdcl commented Nov 23, 2021 •

edited

Loading

0x2b3bfa0 commented Nov 23, 2021 •

edited

Loading

0x2b3bfa0 commented Jan 21, 2022 •

edited

Loading

dacbd commented Feb 22, 2022 •

edited

Loading

dacbd commented Feb 23, 2022 •

edited

Loading