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

refactor(titus): Adding load balancer incompatibility #7386

Merged

Conversation

alanmquach
Copy link
Contributor

@alanmquach alanmquach commented Sep 12, 2019

Removing the need to select a cloud provider when creating a new load balancer when the only providers to be considered are aws and titus since titus delegates to aws anyways.

Probably easiest to review just the last commit (12f6f42) since the first 2 are simply reverts of the current implementation.

Managed to do this without introducing any titus awareness into amazon by allowing incompatible load balancer types to be specified by the provider (in this case titus.module.ts):

    loadBalancer: {
      LoadBalancersTag: AmazonLoadBalancersTag,
      incompatibleLoadBalancerTypes: [
        {
          type: 'classic',
          reason: 'Classic Load Balancers cannot be used with Titus as they do not have IP based target groups.',
        },
      ],
      useProvider: 'aws',
    },

This allows the load balancer selection dialog to generically ask whether choices are compatible.
aquachpy_·_Load_Balancers

Also if any choice is not compatible with all providers (isIncompatibleWithAllProviders), it is removed as a choice altogether.
aquachpy_·_Load_Balancers

Like this better and it removes the extra click.

Also like this because it could be extended in the future to allow app-types (if that becomes a thing) provide opinions about what load balancers it does not want to be configured with.

Not just because I came up with it.

@spinnakerbot
Copy link
Contributor

The following commits need their title changed:

Please format your commit title into the form:

<type>(<scope>): <subject>, e.g. fix(kubernetes): address NPE in status check

This allows us to easily generate changelogs & determine semantic version numbers when cutting releases. You can read more about commit conventions here.

1 similar comment
@spinnakerbot
Copy link
Contributor

The following commits need their title changed:

Please format your commit title into the form:

<type>(<scope>): <subject>, e.g. fix(kubernetes): address NPE in status check

This allows us to easily generate changelogs & determine semantic version numbers when cutting releases. You can read more about commit conventions here.

@alanmquach alanmquach merged commit 7d65ea9 into spinnaker:master Sep 12, 2019
@alanmquach alanmquach deleted the redo-titus-clb-incompatibility branch September 12, 2019 19:40
alanmquach added a commit to alanmquach/deck that referenced this pull request Sep 12, 2019
e0ce768 feat(pager): Looking up multiple apps by name to page (spinnaker#7387)
42c2946 fix(core): ensure filter tag removal removes tags from filter (spinnaker#7212)
733132b refactor(core): reactify the show pipeline history modal (spinnaker#7382)
7d65ea9 refactor(titus): Adding load balancer incompatibility (spinnaker#7386)
3e76186 fix(core/pipeline): exclude correlation IDs (and more) when re-running (spinnaker#7374)
alanmquach added a commit to alanmquach/deck that referenced this pull request Sep 12, 2019
7d65ea9 refactor(titus): Adding load balancer incompatibility (spinnaker#7386)
alanmquach added a commit to alanmquach/deck that referenced this pull request Sep 12, 2019
7d65ea9 refactor(titus): Adding load balancer incompatibility (spinnaker#7386)
alanmquach added a commit that referenced this pull request Sep 12, 2019
e0ce768 feat(pager): Looking up multiple apps by name to page (#7387)
42c2946 fix(core): ensure filter tag removal removes tags from filter (#7212)
733132b refactor(core): reactify the show pipeline history modal (#7382)
7d65ea9 refactor(titus): Adding load balancer incompatibility (#7386)
3e76186 fix(core/pipeline): exclude correlation IDs (and more) when re-running (#7374)
alanmquach added a commit that referenced this pull request Sep 12, 2019
7d65ea9 refactor(titus): Adding load balancer incompatibility (#7386)
alanmquach added a commit that referenced this pull request Sep 12, 2019
7d65ea9 refactor(titus): Adding load balancer incompatibility (#7386)
yunzhangit pushed a commit to yunzhangit/deck that referenced this pull request Mar 28, 2021
yunzhangit pushed a commit to yunzhangit/deck that referenced this pull request Mar 28, 2021
e0ce768 feat(pager): Looking up multiple apps by name to page (spinnaker#7387)
42c2946 fix(core): ensure filter tag removal removes tags from filter (spinnaker#7212)
733132b refactor(core): reactify the show pipeline history modal (spinnaker#7382)
7d65ea9 refactor(titus): Adding load balancer incompatibility (spinnaker#7386)
3e76186 fix(core/pipeline): exclude correlation IDs (and more) when re-running (spinnaker#7374)
yunzhangit pushed a commit to yunzhangit/deck that referenced this pull request Mar 28, 2021
7d65ea9 refactor(titus): Adding load balancer incompatibility (spinnaker#7386)
yunzhangit pushed a commit to yunzhangit/deck that referenced this pull request Mar 28, 2021
7d65ea9 refactor(titus): Adding load balancer incompatibility (spinnaker#7386)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants