Adds environment & version to Deploy events #758
Conversation
| @@ -425,6 +425,9 @@ type DeploymentsCreateOpts struct { | |||
| // Image is the image that's being deployed. | |||
| Image image.Image | |||
|
|
|||
| // Environment is the environment where the image is being deployed | |||
| Environment string | |||
There was a problem hiding this comment.
This is probably ok for now, but it'd probably be better in the long run if we had this on the Empire struct itself, so it works for non-github deployments. If I emp deploy <image>, I'd still expect to see the environment in the event.
That means we'd need an --environment flag.
There was a problem hiding this comment.
Hmm - yeah, I guess actually taking the environment FROM the github deployment isn't actually all that useful. It's not like Empire runs multiple environments. Let me think about that - doesn't seem like it'd need to be super far reaching?
There was a problem hiding this comment.
I think the initial version could just be used for this.
There was a problem hiding this comment.
Ok, took a first stab at addressing this. Let me know what you think.
|
lgtm if you wanna just start with this 👍 |
Rather than rely on the environment from the github deploy, lets just setup an environment for the whole Empire daemon. Right now this is only used in Deploy Events, but this is also the first step to supporting GH-480
| @@ -438,6 +448,10 @@ func (opts DeploymentsCreateOpts) Event() DeployEvent { | |||
| if opts.App != nil { | |||
| e.App = opts.App.Name | |||
| } | |||
|
|
|||
| if opts.Environment != "" { | |||
| e.Environment = opts.Environment | |||
There was a problem hiding this comment.
I don't think environment should be on the DeploymentsCreateOpts at all, you can just set the Environment on the even in the Deploy method:
event.Environment = e.EnvironmentWhich means that you don't need the GetEnvironment() method and the server doesn't need to know anything about passing it.
|
Cool updated - thanks |
| @@ -284,6 +287,10 @@ func (e *Empire) DomainsDestroy(ctx context.Context, domain *Domain) error { | |||
| return nil | |||
| } | |||
|
|
|||
| func (e *Empire) GetEnvironment() string { | |||
|
Nice! Looks like it needs a rebase, but 👍 otherwise. |
|
Oh, can you update the changelog as well? |
|
Done! |
| @@ -9,6 +9,7 @@ | |||
| * It's now possible to lock down the GitHub authorization to a specific team via the `--github.team.id` flag [#745](https://github.com/remind101/empire/pull/745). | |||
| * Empire can now integrate with Conveyor to build Docker images on demand when using the GitHub Deployments integration [#747](https://github.com/remind101/empire/pull/747). | |||
| * Stdout and Stdin from interactive run sessions can now be sent to CloudWatch Logs for longterm storage and auditing [#757](https://github.com/remind101/empire/pull/757). | |||
| * Add `Environment` and `Release` to Deploy Events. `--environment` will likely be used for tagging resource later. [#758](https://github.com/remind101/empire/pull/758) | |||
There was a problem hiding this comment.
Nit, s/resource/resources/g
Adds environment & version to Deploy events
Fixes GH-755