Bcox termination msg #71
Conversation
| } | ||
| assert.Equal(t, tc.success, retVal, msg) | ||
| closer() | ||
| cancel() |
There was a problem hiding this comment.
Nitpicking: usually cancel() should be called via defer right after creating the context. Does not matter much here, since this is just a unit test.
There was a problem hiding this comment.
Since we're doing this in a loop, I wanted the cancel to be called at the end of the test case, not at the end of the function. Not sure what the usual/idiomatic way of handling this in a test is. Do you know how it's usually handled? I suppose deferring til the end of the function would work just as well and cleanup all created contexts when the test finished...
There was a problem hiding this comment.
It doesn't really matter here, even if there was a chance for leaking it, since this is just a unit test. The rule is to always use defer cancel() right after creating it, but rules are meant to be broken...
This adds a termination log file into the unit. This is a bit lighter weight change than mounting a file into the unit (ala the kubelet). In addition to this change, I also allowed all units to enter the terminated state after the unit's process finishes running. This will allow us to implement gracefully falling back to logs for the termination message.
Units need to know more and more configuration info so I've moved securityContext, probes and termination message configuration into a single structure that's written into the unit (
unitConfig)Falling back to logs on error hasn't been implemented yet but I'm not incredibly concerned about that right now. I believe, with this change, we can easily add that later.