add channel to supervisor #259
Conversation
elliott-davis
force-pushed
the
elliott/channels
branch
2 times, most recently
from
1d508b2
to
2e4de95
Compare
There was a problem hiding this comment.
So I had some nits, but I would like to learn about the real purpose behind the channel field. Isn't passing it as a --channel parameter to supervisor pointless? The updates are turned off and we don't want to turn them on, because doing updates behind k8s back is a no-no. That makes --channel a no-op, basically, right?
| @@ -118,6 +118,9 @@ type Service struct { | |||
| // Name is the name of the Habitat service that this Habitat object represents. | |||
| // This field is used to mount the user.toml file in the correct directory under /hab/user/ in the Pod. | |||
| Name string `json:"name"` | |||
| // Channel is the value of the --channel flag for the hab client. | |||
There was a problem hiding this comment.
v1beta1 custom version is frozen and deprecated. The latter wasn't announced yet though, but we are thinking about deprecating it retroactively (#257). So I would drop it.
pkg/apis/habitat/v1beta1/types.go
Outdated
| @@ -140,6 +143,9 @@ type ServiceV1beta2 struct { | |||
| // Name is the name of the Habitat service that this Habitat object represents. | |||
| // This field is used to mount the user.toml file in the correct directory under /hab/user/ in the Pod. | |||
| Name string `json:"name"` | |||
| // Channel is the value of the --channel flag for the hab client. | |||
| // Optional. Defaults to `stable`. | |||
There was a problem hiding this comment.
Please reword this line as two lines:
// Defaults to `stable`.
// +optional
pkg/controller/v1beta1/controller.go
Outdated
| @@ -583,6 +583,13 @@ func (hc *HabitatController) newDeployment(h *habv1beta1.Habitat) (*appsv1beta1. | |||
| "--group", h.Spec.Service.Group) | |||
| } | |||
|
|
|||
| if h.Spec.Service.Channel != "" { | |||
There was a problem hiding this comment.
Ditto about being frozen and deprecated.
| @@ -140,6 +143,9 @@ type ServiceV1beta2 struct { | |||
| // Name is the name of the Habitat service that this Habitat object represents. | |||
| // This field is used to mount the user.toml file in the correct directory under /hab/user/ in the Pod. | |||
| Name string `json:"name"` | |||
| // Channel is the value of the --channel flag for the hab client. | |||
There was a problem hiding this comment.
This documentation doesn't really tell much about the purpose of this field.
|
@krnowak yes we do have supervisor updates turned off so for the supervisor, the action is a no-op. However, the habitat-updater service can use the channel to ask builder if there are any new packages available, then tell kubernetes to orchestrate the upgrade. |
elliott-davis
force-pushed
the
elliott/channels
branch
from
2e4de95
to
bd72c55
Compare
Signed-off-by: Elliott Davis <elliott@excellent.io>
elliott-davis
force-pushed
the
elliott/channels
branch
from
bd72c55
to
d682a5b
Compare
|
LFAD overall I think, but I would like to hear a second opinion. @asymmetric: ^ We used to have a related issue and a PR for it - #126 and #128. In the meantime, could you write a bit better commit message, please? :) Some explanation along the lines of habitat-updater being able to figure out which channel the service should follow on updates. |
Could you explain how this works? How I understand it:
Is that correct? Something along these lines could be a useful description for the PR, cause we are not very familiar with these other pieces that are now building on top of the operator :) |
|
@asymmetric updated! |
There was a problem hiding this comment.
I was thinking of how this affects backwards-compatibility and concluded there shouldn't be problems, since the key is optional and omitempty.
So an existing serialized Habitat object without this key is equivalent to a newer one where the key hasn't been specified. So all good.
Habitat updater is a service that runs inside of a k8's cluster to watch for changes in pods running habitat services and reconcile any updates from builder. It does this by querying the k8's pods api for anything with a
habitat=truelabel. Then it queries the supervisors running in those pods for what services they are running. Once it has a list of services, it asks builder for the most recent version of those packages in the stable channel. This is undesirable if you want to run a version of your package from the dev/acceptance channel.This change will allow you to tell the Habitat supervisor what channel it should be pinned to with the
--channelflag. The update service will have to be modified after this change to get the channel from the supervisor and query builder accordingly.Signed-off-by: Elliott Davis elliott@excellent.io