Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define rationale for Habitat type structure #182

Open
asymmetric opened this issue Feb 7, 2018 · 7 comments
Open

Define rationale for Habitat type structure #182

asymmetric opened this issue Feb 7, 2018 · 7 comments
Labels

Comments

@asymmetric
Copy link
Contributor

It's not totally clear at the moment why we put keys where we put them in the Habitat type.

  • Is there a clear rationale?
  • Would it make more sense to flatten it by removing the Service subtype?

I say this because I'm having trouble justifying why configSecretName should be under Service, rather than under Habitat directly.

Also, every time we have to add a new field, we should clearly know where.


I thought a clear rationale was: every field that represents a flag passed to the supervisor should go under Service. So for example: --name, --topology, --bind.

If that's true, than configSecretName should be move up.

But I wonder if such a rationale makes sense from the point of view of a user. It is not based on a logical grouping, rather it's based on inheritance (of the flags used by an existing tool).

@asymmetric asymmetric mentioned this issue Feb 12, 2018
@asymmetric
Copy link
Contributor Author

I think we probably don't need to have a service key at all, i.e. we can just one level of depth.

@asymmetric
Copy link
Contributor Author

asymmetric commented Feb 20, 2018

@zeenix WDYT? Flattening the type layout will be a breaking change, and you seem to have strong feelings towards backwards compatibilty.

Also /cc @krnowak @lilic

@lilic
Copy link
Contributor

lilic commented Feb 20, 2018

@asymmetric Can you paste a sample manifest file of your idea for better clarity.

Also as a side note according to the API version we are in beta not alpha, so these changes should have been done before that.

@asymmetric
Copy link
Contributor Author

asymmetric commented Feb 20, 2018

Basically,

apiVersion: habitat.sh/v1beta1
kind: Habitat
metadata:
  name: example-configured-habitat
spec:
  image: kinvolk/redis-hab
  count: 1
  service-name: db
  topology: standalone
  group: redisdb
  configSecretName: user-toml

@zeenix
Copy link
Contributor

zeenix commented Feb 20, 2018

well I'm against needless API breakage but I'm not sure it's the case in here. However, we should try our best to design this well right now if we're breaking again and also consult Chef folks about breaking this API and how it affects the k8s and helm exporters.

@asymmetric
Copy link
Contributor Author

asymmetric commented May 7, 2018

After thinking about this some more, I think the current structure is fine, and that configSecretName is just an outlier that is probably misplaced

Basically, the "rationale" IMO is that spec.service is for flags that are passed to the operator. In this sense, configSecretName should've been placed directly under the spec key.

I've opened an issue to investigate whether that makes sense.

@krnowak
Copy link
Contributor

krnowak commented May 8, 2018

As I said in #269, I think that a rationale could be k8s specific stuff vs habitat specific stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants