search

coreos / container-linux-update-operator

A Kubernetes operator to manage updates of Container Linux by CoreOS
Apache License 2.0
209 stars 49 forks source link

Only Mark Node Schedulable If Agent Marked Unschedulable #176

Closed hankjacobs closed 6 years ago

hankjacobs commented 6 years ago

This PR changes the agent to mark a node schedulable only if the agent was responsible for marking it unschedulable.

Prior to this change, the agent would always mark a node as schedulable when starting up. This could result in a node that had been marked unschedulable by an external process (eg. by an admin using kubectl) being marked schedulable by it's update agent if the agent restarted for some reason.

coreosbot commented 6 years ago

Can one of the admins verify this patch?

coreosbot commented 6 years ago

Can one of the admins verify this patch?

gojihotsauce commented 6 years ago

Can one of the admins verify this patch?

gojihotsauce commented 6 years ago

Can one of the admins verify this patch?

gojihotsauce commented 6 years ago

Can one of the admins verify this patch?

coreosbot commented 6 years ago

Can one of the admins verify this patch?

coreosbot commented 6 years ago

Can one of the admins verify this patch?

coreosbot commented 6 years ago

Can one of the admins verify this patch?

coreosbot commented 6 years ago

Can one of the admins verify this patch?

coreosbot commented 6 years ago

Can one of the admins verify this patch?

coreosbot commented 6 years ago

Can one of the admins verify this patch?

coreosbot commented 6 years ago

Can one of the admins verify this patch?

euank commented 6 years ago

Instance of https://github.com/jenkinsci/ghprb-plugin/issues/292.

Honestly, we should probably move CoreOS projects related to k8s over to prow now that they've finished moving upstream off jenkins.

euank commented 6 years ago

ok to test

sdemos commented 6 years ago

In addition to @euank's comments, if you don't mind squashing your cleanup and merge commits together, that would be awesome. Probably just the first three commits you have should cover it pretty well.

Just ping us whenever that is done and we will give it a final review.

coreosbot commented 6 years ago

Can one of the admins verify this patch?

hankjacobs commented 6 years ago

Will do! A bit swamped but have this on my to-do list.

coreosbot commented 6 years ago

Can one of the admins verify this patch?

sdemos commented 6 years ago

@hankjacobs any updates on this? just a friendly ping, no rush.

hankjacobs commented 6 years ago

still on my to-do list. sorry about that! I swear I'll get around to it in the next two weeks.

hankjacobs commented 6 years ago

@euank @sdemos Good to go. Thanks for your patience!

sdemos commented 6 years ago

Thanks for pushing the fix! I'll take a look at this again sometime soon.

My mistake with the issue reference, I had the link in my copy buffer and accidentally put it in a comment on an unrelated issue.

hankjacobs commented 6 years ago

@sdemos All set with the requested changes. I haven't had a chance to test these changes since I had torn down my original testing infrastructure a while back. I may have some time to set it up tomorrow but no guarantees. Would you be able to test? If not, no biggie.

sdemos commented 6 years ago

Changes look good to me. I can test it out. Thanks for the quick fixes!