Dronev06 #33
Dronev06 #33
Conversation
| @@ -135,78 +224,76 @@ func wrapMain() error { | |||
|
|
|||
| e := os.Environ() | |||
| e = append(e, fmt.Sprintf("GOOGLE_APPLICATION_CREDENTIALS=%s", keyPath)) | |||
| runner := NewEnviron(workspace.Path, e, os.Stdout, os.Stderr) | |||
| wd, err := os.Getwd() | |||
There was a problem hiding this comment.
using os.Getwd() instead of workspace.Path seems to work in my limited testing, but I'm new to golang and I would be very surprised if this is the correct/best practice way to handle this, can anyone more fluent in golang please review?
There was a problem hiding this comment.
I did some testing and this works as is, however we can also just remove wd as well and use "" instead.
This is used to set the command runner's environment:
// Dir specifies the working directory of the command. // If Dir is the empty string, Run runs the command in the // calling process's current directory. Dir string
| "repo": repo, | ||
| "build": build, | ||
| "system": system, | ||
| // TODO do we really need these? |
There was a problem hiding this comment.
I don't know if these are needed -- it would allow people to reference variables in kube templates that aren't explicitly called out in the pipeline configuration. I think we should not allow that. If a pipeline author wants to reference this data, they should explicitly pass it in as a var in .drone.yml
| vargs.Template: data, | ||
| vargs.SecretTemplate: secrets, | ||
| kubeTemplate: data, | ||
| secretTemplate: secretsAndData, |
There was a problem hiding this comment.
This is a significant difference in behavior with the old version. Previously, only secrets were passed to secret templates, now secret templates have access to both variables and secrets.
| fmt.Println("Skipping kubectl apply, because dry_run: true") | ||
| return nil | ||
| } | ||
|
|
||
| // Set the execution namespace. | ||
| if len(vargs.Namespace) > 0 { | ||
| fmt.Printf("Configuring kubectl to the %s namespace\n", vargs.Namespace) | ||
| if len(c.String("namespace")) > 0 { |
There was a problem hiding this comment.
I did NO TESTING of the namespace feature.
|
Really looking forward to using this when ready. What's holding up the merge at the moment? |
|
Hi @schugh, a few other things came up in our priorities, but we are back to working on the upgrade now. Hope to have things ready for users soon! |
|
Thanks for the update! looking forward to taking it for a test drive. |
| @@ -0,0 +1,9 @@ | |||
| #v0.60 tag (tagging strategy TBD) | |||
There was a problem hiding this comment.
My proposal is we follow a similar versioning scheme as np internally:
Tag the current versions as: 0.4 and 0.4.0
Release when this is merged: 0.6.0
latest -> master
stable -> latest tag
| @@ -135,78 +224,76 @@ func wrapMain() error { | |||
|
|
|||
| e := os.Environ() | |||
| e = append(e, fmt.Sprintf("GOOGLE_APPLICATION_CREDENTIALS=%s", keyPath)) | |||
| runner := NewEnviron(workspace.Path, e, os.Stdout, os.Stderr) | |||
| wd, err := os.Getwd() | |||
There was a problem hiding this comment.
I did some testing and this works as is, however we can also just remove wd as well and use "" instead.
This is used to set the command runner's environment:
// Dir specifies the working directory of the command. // If Dir is the empty string, Run runs the command in the // calling process's current directory. Dir string
|
I changed the base for this to |
There are still a few open questions I'll call out in comments.
I also haven't thought about versioning and tagging yet. I think we need to two branches (or tags) to indicate the most up-to-date versions of the v0.4 and v0.5+ style of the plugin.