-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Support non-public cloud environments in the Azure Service Bus scaler #1907
Conversation
52972e7
to
c8604a0
Compare
@@ -249,6 +262,7 @@ func (s *azureServiceBusScaler) getServiceBusNamespace() (*servicebus.Namespace, | |||
namespace.Name = s.metadata.namespace | |||
} | |||
|
|||
namespace.Suffix = s.metadata.endpointSuffix |
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.
I think this is all it takes for the servicebus client to work in other cloud environments, hope I didn't miss anything.
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.
@ahmelsayed Can you confirm this?
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.
sorry for the delay, I've been out for a while and just got back.
There is also namespace.Environment
that I see in the API. I don't have an easy way to validate it, but I'll try to get an environment where I can.
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.
From what I could tell, namespace.Environment
is only used for getting the service bus resource ID when calling NamespaceWithEnvironmentBinding to configure a namespace using environment details. Since we either create the namespace with connection string or with pod identity, I think namespace.Environment
isn't required. That also makes sense to me because as far as I know the service bus client should also support cloud environments that are not the publicly known clouds, like air gapped clouds.
@ahmelsayed I might be able to test it in one of our clusters in US Gov if that helps.
@@ -249,6 +262,7 @@ func (s *azureServiceBusScaler) getServiceBusNamespace() (*servicebus.Namespace, | |||
namespace.Name = s.metadata.namespace | |||
} | |||
|
|||
namespace.Suffix = s.metadata.endpointSuffix |
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.
@ahmelsayed Can you confirm this?
Signed-off-by: amirschw <24677563+amirschw@users.noreply.github.com>
658fa73
to
ecb2047
Compare
@ahmelsayed We are postponing the v2.4 release to tomorrow so you can give a quick review. If there are changes to be made, would you mind doing them so we can ship, please? |
…#1907) Signed-off-by: amirschw <24677563+amirschw@users.noreply.github.com> Signed-off-by: Tsuyoshi Ushio <ushio@simplearchitect.com>
…kedacore#1907) Signed-off-by: amirschw <24677563+amirschw@users.noreply.github.com> Signed-off-by: nilayasiktoprak <nilayasiktoprak@gmail.com>
Similarly to #1863, added an optional
cloud
parameter to the Azure SB scaler to support non-public Azure clouds.PR for updating the documentation: kedacore/keda-docs#473.
Fixes #1918
Checklist
Signed-off-by: amirschw 24677563+amirschw@users.noreply.github.com