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

[sup] Introduce optional application environment qualifier on Service Group #2791

Merged
merged 1 commit into from
Jul 21, 2017

Conversation

fnichol
Copy link
Collaborator

@fnichol fnichol commented Jul 20, 2017

This change adds a new optional qualifier to a Service Group string,
called an Application Environment. The printed representation of a
Service Group with Application Environment delimited with a hash
character and the application name and environment name are delimited
with a period character. For example, a Service Group containing an
application name "twitter" and environment name of "production" would
look like:

twitter.production#redis.cache

The components of the Service Group are as follows:

    twitter   . production  #  redis  . cache
|-application-|-environment-|-service-|-group-|

To load a service, 2 new options are added which are used together
(--application/-a and --environment/-e). For example:

hab svc load core/redis \
  --group cache \
  --application twitter \
  --environment production

@thesentinels
Copy link
Contributor

Thanks for the pull request! Here is what will happen next:

  1. Your PR will be reviewed by the maintainers
  2. If everything looks good, one of them will approve it, and your PR will be merged.

Thank you for contributing!

…Group.

This change adds a new optional qualifier to a Service Group string,
called an Application Environment. The printed representation of a
Service Group with Application Environment delimited with a hash
character and the application name and environment name are delimited
with a period character. For example, a Service Group containing an
application name "twitter" and environment name of "production" would
look like:

    twitter.production#redis.cache

The components of the Service Group are as follows:

        twitter   . production  #  redis  . cache
    |-application-|-environment-|-service-|-group-|

To load a service, 2 new options are added which are used together
(`--application/-a` and `--environment/-e`). For example:

    hab svc load core/redis \
      --group cache \
      --application twitter \
      --environment production

Signed-off-by: Fletcher Nichol <fnichol@nichol.ca>
@reset
Copy link
Collaborator

reset commented Jul 21, 2017

@fnichol per our conversation on the walk over to the stadium, I think we should swap this to a single flag and just go with a naming convention of including a dot - that way this becomes a) tagging mechanism that visualizers of ring data can leverage and b) potentially something we can leverage as a small world in our gossip protocol at a later date

@reset
Copy link
Collaborator

reset commented Jul 21, 2017

Actually, after thinking this over and then having a chat with @adamhjk about it this morning I think this is the right way to go. I think we are actually overloading the concept of "group" and these acting as more than just a label is the right way to go. Moving forward I think we should restructure things like this:

  • Application - a collection of services working together to provide an experience
  • Environment - an application providing an experience in a logical environment (acceptance, live, etc)
  • Group - a grouping of same services within an application. This could be something like blue or green. I think we will more often then not see group just as "default" if we make this change

What is also great about this change is we finally have an actual solution for how to assemble a "small world" in the gossip protocol for garbage collection and faster spreading of rumors at scale.

Gonna do a code review on this one last time and then get it merged in.

@reset
Copy link
Collaborator

reset commented Jul 21, 2017

@fnichol this looks perfect as is, we can pipe things into the gossip layer and make any necessary changes in a future commit!

tenor-228610679

@reset
Copy link
Collaborator

reset commented Jul 21, 2017

@thesentinels approve

@thesentinels
Copy link
Contributor

🤘 I am testing your branch against master before merging it. We do this to ensure that the master branch is never failing tests.

@thesentinels
Copy link
Contributor

:neckbeard: Travis CI has started testing this PR.

@thesentinels
Copy link
Contributor

💔 Travis CI reports this PR failed to pass the test suite.

The next step is to examine the job and figure out why. If it is transient, you can try re-triggering the Travis CI Job - if it passes, this PR will be automatically merged. If it is not transient, you should fix the issue and update this pull request, and issue approve again. If you believe it will never pass, and you are feeling :godmode:, you can issue a force to merge this PR anyway.

@reset reset merged commit f4120fc into master Jul 21, 2017
@reset reset deleted the fnichol/sup-add-app-id branch July 21, 2017 22:53
@eeyun eeyun added I-linux and removed P-linux labels Mar 6, 2018
raskchanky pushed a commit that referenced this pull request Apr 16, 2019
[sup] Introduce optional application environment qualifier on Service Group
@christophermaier christophermaier added Focus:Supervisor Related to the Habitat Supervisor (core/hab-sup) component Platform: Linux Deals with Linux-specific behavior and removed A-supervisor labels Jul 24, 2020
@christophermaier christophermaier added Platform: Windows Deals with Windows-specific behavior Platform: macOS Deals with macOS-specific behavior Type: Feature Issues that describe a new desired feature and removed I-linux labels Jul 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Focus:Supervisor Related to the Habitat Supervisor (core/hab-sup) component Platform: Linux Deals with Linux-specific behavior Platform: macOS Deals with macOS-specific behavior Platform: Windows Deals with Windows-specific behavior Type: Feature Issues that describe a new desired feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants