search

josmo / drone-rancher

Drone plugin for triggering Rancher deployments
53 stars 35 forks source link

Update plugin to go-rancher/v2 to allow retention of secrets on upgrade #27

Closed 0rax closed 6 years ago

0rax commented 6 years ago

Hi, I have been using your plugin for quite a while now and stumbled upon an issue with newer version of Rancher when using Rancher secrets.

When upgrading a service containing a launch configuration incompatible with the API v1 (exposed as the github.com/rancher/go-rancher/client/ package) such as secrets the plugin will not be able to retrieve those in the LaunchConfiguration object.

This can be fixed by using the API v2 Package (github.com/rancher/go-rancher/v2) which allow us to retrieve those fields and make sure that no information is lost on upgrade. It seems that a lot of new fields are affected by that and updating the API is a good move.

This PR include the move to this new version of the API as well as fixing some of the linter warnings and some minor changes in how the software logs (-log.Info(fmt.Sprintf()) +log.Infof()). I also took the liberty to resolve one of the TODO about how to better manage pagination & all by using filter argument from the service name and stackId.

PS: The v2 API is defined since Rancher 1.2

josmo commented 6 years ago

Thanks @0rax Seems reasonable :) I'll take a closer look in the next couple days and merge.

Vesli commented 6 years ago

Hey this is really interesting, I just switched to drone / rancher and intends to bring it for my whole company, and this PR would be of great needs. Any news about it @josmo ?

peetasan commented 6 years ago

Hi, we were just about to fix the very same issue when we stumbled upon this PR. It would be very useful for us once merged.

josmo commented 6 years ago

Ok I'll get it merged today

Sent from mobile

On Nov 21, 2017 07:42, "Peter Vajda" notifications@github.com wrote:

Hi, we were just about to fix the very same issue when we stumbled upon this PR. It would be very useful for us once merged.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/josmo/drone-rancher/pull/27#issuecomment-346015315, or mute the thread https://github.com/notifications/unsubscribe-auth/AAXrmG6_4n9lAR93NqeIOs8mC6UzxcwZks5s4sVRgaJpZM4QG27S .

josmo commented 6 years ago

Thanks for the PR @0rax and sorry it took so long to get it merged in!