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

Support non-public cloud environments in the Azure Service Bus scaler #1907

Merged
merged 1 commit into from
Aug 5, 2021

Conversation

amirschw
Copy link
Contributor

@amirschw amirschw commented Jun 27, 2021

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

  • Commits are signed with Developer Certificate of Origin (DCO - learn more)
  • Tests have been added
  • A PR is opened to update the documentation on (repo, PR) (if applicable)
  • Changelog has been updated

Signed-off-by: amirschw 24677563+amirschw@users.noreply.github.com

@@ -249,6 +262,7 @@ func (s *azureServiceBusScaler) getServiceBusNamespace() (*servicebus.Namespace,
namespace.Name = s.metadata.namespace
}

namespace.Suffix = s.metadata.endpointSuffix
Copy link
Contributor Author

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.

Copy link
Member

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?

Copy link
Contributor

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.

Copy link
Contributor Author

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.

CHANGELOG.md Outdated Show resolved Hide resolved
@@ -249,6 +262,7 @@ func (s *azureServiceBusScaler) getServiceBusNamespace() (*servicebus.Namespace,
namespace.Name = s.metadata.namespace
}

namespace.Suffix = s.metadata.endpointSuffix
Copy link
Member

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>
@zroubalik zroubalik added this to the v2.4.0 milestone Jul 21, 2021
@tomkerkhove
Copy link
Member

@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?

@ahmelsayed ahmelsayed merged commit d7f7f09 into kedacore:main Aug 5, 2021
TsuyoshiUshio pushed a commit that referenced this pull request Aug 6, 2021
…#1907)

Signed-off-by: amirschw <24677563+amirschw@users.noreply.github.com>
Signed-off-by: Tsuyoshi Ushio <ushio@simplearchitect.com>
nilayasiktoprak pushed a commit to nilayasiktoprak/keda that referenced this pull request Oct 23, 2021
…kedacore#1907)

Signed-off-by: amirschw <24677563+amirschw@users.noreply.github.com>
Signed-off-by: nilayasiktoprak <nilayasiktoprak@gmail.com>
@amirschw amirschw deleted the servicebus-endpoint-suffix branch November 20, 2021 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support non-public clouds for Azure Service Bus
4 participants