-
Notifications
You must be signed in to change notification settings - Fork 900
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
fix(titus): Removing NLB as it is not compatible with Titus #7317
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks ok to me, though it feels a bit gross to put user facing Titus-aware stuff in the Amazon provider. I suspect you'd have to architect some pretty involved abstraction thing to avoid that so I have no issue with it as-is.
<div className="alert alert-warning"> | ||
<p> | ||
<i className="fa fa-exclamation-triangle" /> Note: Classic Load Balancers cannot be used with Titus as | ||
they do not have <em>ip</em> based target groups. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they do not have <em>ip</em> based target groups. | |
they do not have <em>IP</em> based target groups. |
Co-Authored-By: Erik Munson <erik@ipsumcreative.com>
Yeah, I don't like that either. Looking to see if there's an easy way to fork out and have this live in Titus instead. |
{ | ||
CreateLoadBalancerModal: TitusLoadBalancerChoiceModal, | ||
}, | ||
CloudProviderRegistry.getProvider('aws').loadBalancer, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@erikmunson I like this better, but I have mixed feelings about getting the the aws loadBalancer object from the CloudProviderRegistry
vs importing it directly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ugghhhh, this is not great either because now for applications with both aws
and titus
configured, users are now asked to pick a provider because titus
is not 100% delegated to aws
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tiny typo but otherwise seems very reasonable, we had a conversation about whether to filter out the CLB option altogether and will leave that to your judgement.
@@ -5,21 +5,29 @@ import { ILoadBalancerModalProps, ModalClose, ReactModal, noop } from '@spinnake | |||
|
|||
import { IAmazonLoadBalancerConfig, LoadBalancerTypes } from './LoadBalancerTypes'; | |||
|
|||
export interface IAmazonLoadBalancerChoiceModalDetailsInectedProps { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/Inected/Injected
373f581 fix(titus): Removing NLB as it is not compatible with Titus (spinnaker#7317) f0d3254 refactor(titus): Making service job processes human friendly (spinnaker#7350)
* chore(core): Bump version to 0.0.405 6a47557 refactor(core): Reactify rename pipeline modal (#7368) 0d88644 feat(notifications): Add additional fields for Github and get config from halyard (#7239) 902c0b3 refactor(core): use a fieldLayout for manualExecution (#7356) 9f5a860 fix(core/pipeline): fix date picker for manual execution parameters where constraint == 'date' 744cf19 feat(core/presentation): Add DayPickerInput form input component 5a354b1 feat(core): improve readability of pipeline cancellation message (#7369) b889857 fix(artifacts): fix uncaught undefined exception with artifacts (#7362) dc16166 fix(core): Fix shadowed var usage (#7367) 5d1facd refactor(core): Reactify lock and unlock pipeline modal (#7366) d131e01 fix(core): separate multiple task errors by newline (#7355) 450d681 refactor(core): Reactify enable pipeline modal (#7360) 5155ba1 refactor(core): Reactify disable pipeline modal (#7358) 14035ae fix(kustomize): fix artifact selector (#7359) 12f2eaa refactor(core): Reactify delete pipeline modal (#7357) 48b7195 fix(kubernetes): add StageFailureMessage to Bake Manifest execution details (#7354) 5fae6ef feat(core): add stage status precondition type (#7348) f42977f fix(core): pipeline execution was not displaying all resolvedArtifacts (#7353) 251250b fix(artifacts/k8s): fix rewrite k8s artifact edit (#7352) * chore(titus): Bump version to 0.0.109 373f581 fix(titus): Removing NLB as it is not compatible with Titus (#7317) f0d3254 refactor(titus): Making service job processes human friendly (#7350)
373f581 fix(titus): Removing NLB as it is not compatible with Titus (spinnaker#7317)
* chore(core): Bump version to 0.0.406 d8f1a51 fix(triggers): Protecting from undefined triggers (#7377) ad4c144 feat(core/managed): Add resource dropdown with links to history + source JSON (#7376) 3665d3e fix(typos): fix a few typos in help messages (#7373) * chore(amazon): Bump version to 0.0.209 373f581 fix(titus): Removing NLB as it is not compatible with Titus (#7317)
…pinnaker#7317)" This reverts commit 373f581.
* chore(core): Bump version to 0.0.405 6a47557 refactor(core): Reactify rename pipeline modal (spinnaker#7368) 0d88644 feat(notifications): Add additional fields for Github and get config from halyard (spinnaker#7239) 902c0b3 refactor(core): use a fieldLayout for manualExecution (spinnaker#7356) 9f5a860 fix(core/pipeline): fix date picker for manual execution parameters where constraint == 'date' 744cf19 feat(core/presentation): Add DayPickerInput form input component 5a354b1 feat(core): improve readability of pipeline cancellation message (spinnaker#7369) b889857 fix(artifacts): fix uncaught undefined exception with artifacts (spinnaker#7362) dc16166 fix(core): Fix shadowed var usage (spinnaker#7367) 5d1facd refactor(core): Reactify lock and unlock pipeline modal (spinnaker#7366) d131e01 fix(core): separate multiple task errors by newline (spinnaker#7355) 450d681 refactor(core): Reactify enable pipeline modal (spinnaker#7360) 5155ba1 refactor(core): Reactify disable pipeline modal (spinnaker#7358) 14035ae fix(kustomize): fix artifact selector (spinnaker#7359) 12f2eaa refactor(core): Reactify delete pipeline modal (spinnaker#7357) 48b7195 fix(kubernetes): add StageFailureMessage to Bake Manifest execution details (spinnaker#7354) 5fae6ef feat(core): add stage status precondition type (spinnaker#7348) f42977f fix(core): pipeline execution was not displaying all resolvedArtifacts (spinnaker#7353) 251250b fix(artifacts/k8s): fix rewrite k8s artifact edit (spinnaker#7352) * chore(titus): Bump version to 0.0.109 373f581 fix(titus): Removing NLB as it is not compatible with Titus (spinnaker#7317) f0d3254 refactor(titus): Making service job processes human friendly (spinnaker#7350)
* chore(core): Bump version to 0.0.406 d8f1a51 fix(triggers): Protecting from undefined triggers (spinnaker#7377) ad4c144 feat(core/managed): Add resource dropdown with links to history + source JSON (spinnaker#7376) 3665d3e fix(typos): fix a few typos in help messages (spinnaker#7373) * chore(amazon): Bump version to 0.0.209 373f581 fix(titus): Removing NLB as it is not compatible with Titus (spinnaker#7317)
No description provided.