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

Feature/service bus namespace create if not exists #95

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Feature/service bus namespace create if not exists #95

wants to merge 8 commits into from

Conversation

HenrikSommer
Copy link

Added support for usage of existing Service Bus Namespaces

Creation of Service Bus Namespace is a timeconsuming task in Azure. This pull request addresses this issue with support for usage of existing Service Bus Namespace

  • When a named Service Bus Namespace exists within resource group it can be used as is
  • When a named Service Bus Namespace does not exist within resource group it will be created

Addresses #94

xnetsrak and others added 3 commits June 3, 2021 15:58
Use ServiceBusProvisioningMode CreateIfNotExists for
automatic Service Bus Namespace creation.

Service Bus Namespace creation is a slow process, thus
reuse existing Namespace will highly speed up the
creation process.
@swisslife-bot
Copy link

swisslife-bot commented Jun 4, 2021

CLA assistant check
All committers have signed the CLA.

@glucaci
Copy link
Member

glucaci commented Jun 28, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@sonarcloud
Copy link

sonarcloud bot commented Jun 28, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@glucaci glucaci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good, only one thing I would change (see comment).


private async Task<bool> CheckServiceBusNamespaceExists(string serviceBusNamespace)
{
var serviceBusNamespaces = new List<SBNamespace>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insead of listing all the service bus namespaces I would use _client.Namespaces.CheckNameAvailabilityMethodAsync

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, and we have been there as well. The reason why we didn't go with _client.Namespaces.CheckNameAvailabilityMethodAsync is that it will return the global availability of a name. In this context, we need to know the availability of a name given a resource group.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ServiceBus namespace name must be global available because has a public endpoint.
https://docs.microsoft.com/en-us/rest/api/servicebus/create-namespace ...the name must be unique across Azure to be successfully created

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glucaci did you see my comment here?

Namespace-method with one parameter is reintroduced to preserve backward compatibility.

private async Task<bool> CheckServiceBusNamespaceExists(string serviceBusNamespace)
{
var serviceBusNamespaces = new List<SBNamespace>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ServiceBus namespace name must be global available because has a public endpoint.
https://docs.microsoft.com/en-us/rest/api/servicebus/create-namespace ...the name must be unique across Azure to be successfully created

@glucaci
Copy link
Member

glucaci commented Jul 13, 2021

I will change the namespace checking to _client.Namespaces.CheckNameAvailabilityMethodAsync, because the name must be global available to can create it doesn't matter in which resource group. Or you have another reason to iterate through all the namespaces in a resource which I'm trying to avoid.

@glucaci
Copy link
Member

glucaci commented Aug 31, 2021

@HenrikSommer @Sondergaard do you need any support to finish this one?

@HenrikSommer
Copy link
Author

HenrikSommer commented Sep 10, 2021

I will change the namespace checking to _client.Namespaces.CheckNameAvailabilityMethodAsync, because the name must be global available to can create it doesn't matter in which resource group. Or you have another reason to iterate through all the namespaces in a resource which I'm trying to avoid.

@glucaci Let me give you an example from my own use of the Azure ServiceBus cloud provider. Creating the ServiceBus Namespace is a slow process, whereas topic and subscription are much faster. To speed up our test run we would like to use a named ServiceBus Namespace. I addition we would like to run our integration tests in a pull request gate, in which we cannot assume that the Service Bus Namespace is created. This is the essence of this pull request. In order to do this, we need to know that the ServiceBus Namespace is present within our own resource group. In this case, it is actually of minor importance to check that the ServiceBus Namespace is available globally because:

  • if it is available globally then we will create it anyway
  • if it is not available globally then we cannot create the expected ServiceBus Namespace, and our tests need to fail. In this case, the exception from Azure telling us that the ServiceBus Namespace isn't available is just fine

If you still think we need to check for global availability, then I suggest a hybrid solution with both global and in resource group check.

@michaelstaib
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@sonarcloud
Copy link

sonarcloud bot commented Nov 1, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

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.

6 participants