Disable cron for "Stopped" services #16
Conversation
|
I don't think there is a need for changing it from a pointer - you might've pulled up a newer version (or had one locally), but Otherwise, this looks good, I'll check it out locally in a bit (few days). |
|
Not sure if you got the above message @jmloeffler |
|
Yes. Sorry. I misunderstood and thought that I was to await feedback after you checked things out. I'll take another look at the pointer change I made and report back with a new commit or more info. |
|
I added a commit that I think reverts/fixes the pointer issues. I am a little confused about go vendoring, however. It looked like you were using govendor so I did a |
|
OK i found the problem, in the vendor file I was not referencing the |
| } | ||
|
|
||
| cattleService, err := m.CattleClient.GetServiceByUUID(service.UUID) | ||
| if err == nil { |
There was a problem hiding this comment.
i would combine, if err == nil && cattleService.State == "inactive" {}
|
|
||
| return &schedules, nil | ||
| // GetServices returns the services from the metadata client | ||
| func (m *Client) GetServices() ([]metadata.Service, error) { |
There was a problem hiding this comment.
this might not be used anywhere now.
There was a problem hiding this comment.
It is still being called from here to get the original list of services with the cron label.
https://github.com/jmloeffler/rancher-cron/blob/master/scheduler/scheduler.go#L34
Added logic to look up the service in the cattle api and ignore it if it is stopped/inactive. This required calling into the cattle api for looking up the service state. It didn't make sense for the metadata wrapper to depend on the cattle wrapper so I broke logic out into a new scheduler.
|
I rebased and fixed the excessive nesting based on your feedback. The metadata client call is still being used so I left that in place. Let me know if you think more changes are needed. Thanks for clarifying the govendor stuff. There seems to be a lot of different ways to do vendoring in Go so I assumed I was just doing something wrong. |
|
@smazurov any update on this? Looks like a good feature to merge in |
|
yeah for sure, this is good to go, i need to take a closer look at it, and a few other PRs and features and get a 1.0 out. I have some time set aside this week, so I do hope to get to it soon. I 100% appreciate the contribution! |
I took a stab at #9. First, I updated the metadata.go since the rancher metadata api had changed slightly and broke this (NewClientAndWait no longer returns a pointer). In the second commit, I made it so that GetCronSchedules looks up the service in the cattle service to get the service state. If the state is inactive it ignores it and allows it to be removed from the schedule. The scheduling used to happen in metadata.go but it didn't make sense for metadata.go to depend on cattle.go so I introduced a new scheduler.go. This now contains all of the scheduling logic, leaving metadata.go and cattle.go as thin wrappers around those respective rancher services.
I tested this with Rancher releases 1.1.2 and 1.2.0 and it seems to work ok for both.
I'm new to go, so please let me know if there are any changes you want.