cloudfoundry-incubator / eirinix

Extensions Library for Cloud Foundry Eirini
Apache License 2.0
2 stars 5 forks source link

Proposal: support source_type=STG too #12

Closed drnic closed 4 years ago

drnic commented 5 years ago

Staging pods have label source_type=STG.

Thoughts on how we support EiriniX extn to handle these containers too?

Currently we have a single Handle hook for only source_type=APP. Perhaps we rename it to HandlePodApp (and support deprecated Handle that calls HandlePodApp)

Then we add a new HandlePodStaging for source_type=STG?

/cc @mudler

mudler commented 5 years ago

Currently it is possible to disable the filter at all, so it would be called also for all pods in the eirini namespace, but I am completely in favor of this. Maybe an option to the manager to be consistent and avoid the interface to request a specific handle?

drnic commented 5 years ago

Are you suggesting we more liberally call Handle (e.g. all pods in namespace, or perhaps all pods with either source_type=STG or source_type=APP) and leave it to the Handle() to decide if they care about this pod and/or any of its containers? If so, sounds good.

mudler commented 5 years ago

Are you suggesting we more liberally call Handle (e.g. all pods in namespace, or perhaps all pods with either source_type=STG or source_type=APP) and leave it to the Handle() to decide if they care about this pod and/or any of its containers? If so, sounds good.

Yes! This is already possible, https://godoc.org/github.com/SUSE/eirinix#ManagerOptions

  // FilterEiriniApps enables or disables Eirini apps filters.  Optional, defaults to true
    FilterEiriniApps *bool

See for example the loggregator bridge: https://github.com/SUSE/eirini-loggregator-bridge/blob/master/cmd/root.go#L31 - https://github.com/SUSE/eirini-loggregator-bridge/blob/master/cmd/root.go#L37 , which gets the log for both staging pods and standard apps

drnic commented 5 years ago

Bonus question: why FilterEiriniApps *bool instead of FilterEiriniApps bool?

mudler commented 5 years ago

Bonus question: why FilterEiriniApps *bool instead of FilterEiriniApps bool?

By default FilterEiriniApps is set to true only if nothing was explicitly set. In that way we know if something was really being passed or not