feat: tolerate failed checkins / ready #392
Conversation
Making checkin process optional (default to activated)
cmd/kg/main.go
Outdated
| @@ -134,6 +135,7 @@ var ( | |||
|
|
|||
| func init() { | |||
| cmd.Flags().StringVar(&backend, "backend", k8s.Backend, fmt.Sprintf("The backend for the mesh. Possible values: %s", availableBackends)) | |||
| cmd.Flags().BoolVar(&checkIn, "check-in", true, "Should kilo regularly check-in in backend") | |||
There was a problem hiding this comment.
| cmd.Flags().BoolVar(&checkIn, "check-in", true, "Should kilo regularly check-in in backend") | |
| cmd.Flags().BoolVar(&checkIn, "check-in", true, "Should Kilo regularly check in with the backend") |
cmd/kgctl/main.go
Outdated
| @@ -119,6 +120,7 @@ func main() { | |||
| SilenceErrors: true, | |||
| } | |||
| cmd.PersistentFlags().StringVar(&backend, "backend", k8s.Backend, fmt.Sprintf("The backend for the mesh. Possible values: %s", availableBackends)) | |||
| cmd.PersistentFlags().BoolVar(&checkIn, "check-in", true, "Should kilo consider check-in (LastSeen) in backend") | |||
There was a problem hiding this comment.
| cmd.PersistentFlags().BoolVar(&checkIn, "check-in", true, "Should kilo consider check-in (LastSeen) in backend") | |
| cmd.PersistentFlags().BoolVar(&checkIn, "check-in", true, "Should Kilo prune nodes that have not checked in with the backend") |
| var checkedIn bool | ||
| if (n != nil) && (n.Key != wgtypes.Key{}) && (n.Subnet != nil) && (n.CheckLastSeen) { | ||
| checkedIn = time.Now().Unix()-n.LastSeen < int64(checkInPeriod)*2/int64(time.Second) | ||
| } else { | ||
| checkedIn = true | ||
| } |
There was a problem hiding this comment.
This seems to add more conditions to the check-in logic.
Let's keep this scoped to the change at hand for now. If we need to add more safety let's consider that in a different PR / issue.
| var checkedIn bool | |
| if (n != nil) && (n.Key != wgtypes.Key{}) && (n.Subnet != nil) && (n.CheckLastSeen) { | |
| checkedIn = time.Now().Unix()-n.LastSeen < int64(checkInPeriod)*2/int64(time.Second) | |
| } else { | |
| checkedIn = true | |
| } | |
| var checkedIn bool | |
| if n.CheckLastSeen { | |
| checkedIn = time.Now().Unix()-n.LastSeen < int64(checkInPeriod)*2/int64(time.Second) | |
| } else { | |
| checkedIn = true | |
| } |
There was a problem hiding this comment.
Those tests are here to protect against nil values.
They do not add any logic as they are also in the return statement and would return false anyway.
That was my first implem, but it did not pass the tests. I agree it is not pretty now (return one liner), but did not wanted to refactor too much.
There was a problem hiding this comment.
Agh yes, I woke up in the middle of the night today and literally said "I understand now". I have a little refactor in mind that might be more legible
Making checkin process optional (default to activated)
We quite often have apiserver issues which does not allow some pods to update their last-seen annotation, thus some kilo mesh nodes going "unready" and flapping wireguard configuration(s).
With this setup (currently in test our side), we have a more static setup over time. It also reduces calls to apiserver.
I do not see any drawback, but maybe I just lack imagination 😉