Active Message Count

[kpfaulkner]

Hi

I've created change so when listing/getting queues/topics the CountDetails are populated correctly. The code works fine but I'm trying to sort out the tests.

Currently (even without my change) if I try: "go test" I get the following error:

azure-service-bus-go

namespace_test.go:32:2: use of internal package not allowed
FAIL azure-service-bus-go [setup failed]

I see in namespace_test.go the import "github.com/Azure/azure-service-bus-go/internal/test"

Is there a special way to run tests? I want to get this PR submitted but obviously want to have tests included.

Thanks

Ken

[devigned]

That's pretty odd. There shouldn't be any special way to run tests.

I think you are running into golang/go#21710. Have you run dep ensure prior to running go test?

Also, you can run make test to do all of the things.

[kpfaulkner]

I hadn't run dep ensure previously, but I have now with no difference. Exact same result when I run go test.
Just to clarify I'm on go 1.10 on Windows (will Windows make a difference here?)

Thoughts?

[vcabbage]

Sounds like you might have the fork under your own username in your GOPATH.

If that's the case, it will attempt to import the test package from GOPATH/src/github.com/Azure/azure-service-bus-go/internal/test instead of GOPATH/src/github.com/[username]/azure-service-bus-go/internal/test, which isn't allowed because it's internal.

With Go, it's common to add your fork as a git remote under the official project in your GOPATH to avoid this issue. This post describes it in detail https://blog.sgmansfield.com/2016/06/working-with-forks-in-go/.

[kpfaulkner]

yes I have the fork under my own username (that's how I usually do it). Can't say I've run into this issue before, but will do as you suggest. Thanks for the info!

[kpfaulkner]

hmmm no luck. Hey, if I completely ignore my own fork and just wanted to clone the existing repo and run the tests... should a clone + go test work? Or would I still need to do something else? Since even that's not working for me. (but works for plenty of other repos). Admittedly they dont have "internal"

[devigned]

@kpfaulkner I'll do this on a clean machine today and see if I can replicate your experience.

One item of note, if you look at the TravisCI builds, those clone and run tests from a clean start each time.

[devigned]

@kpfaulkner I've recruited @marstr and @t-jaelli to try to replicate your experience and provide some feedback.

[marstr]

I've just finished running on my box :)

cd $GOPATH/src/github.com/Azure
git clone https://github.com/Azure/azure-service-bus-go.git
cd azure-service-bus-go
dep ensure
git remote add marstr https://github.com/marstr/azure-service-bus-go.git
git fetch marstr
export SERVICEBUS_CONNECTION_STRING=[REDACTED]
go test ./...

Doing this, I didn't run into build errors that you saw above. :)

[t-jaelli]

I think I was able to recreate your issue. I cloned from my fork of the azure-service-bus-go repo.

cd $GOPATH/src/github.com/t-jaelli
git clone https://github.com/t-jaelli/azure-service-bus-go.git
cd azure-service-bus-go
dep ensure
go test ./...

I received the same error. Having both your fork and the upstream in your gopath confuses the compiler.

[devigned]

Thank you @t-jaelli and @marstr!

[kpfaulkner]

@marstr ahh that IS different to how I usually develop. So you actually do your mods within src/github.com/Azure directory? I always have mine as a subdir of src. So yes, I'd have 2 in my GOPATH, one is the original in src/github.com/azure/whatever and MINE would be src/whatever.... hmm never seen the situation where people have it set out like that.

I definitely got further, but now need to gather up client, tenant, subscription ids etc for the tests to run (assuming you already had those setup?)

[marstr]

So yes, I'd have 2 in my GOPATH, one is the original in src/github.com/azure/whatever and MINE would be src/whatever.... hmm never seen the situation where people have it set out like that.

I just always have to be a little different! ;) But really, it's because I hit the problem you're running into all of the time. All of my projects use dep or glide anyway, so the copy that's available from my GOPATH hardly matters at all.

I definitely got further, but now need to gather up client, tenant, subscription ids etc for the tests to run (assuming you already had those setup?)

Ahh yeah, I often forget how much setup I've done for my local machine. I created myself a web application app-registration and use it's service principal in my environment variables AZURE_CLIENT_ID and AZURE_CLIENT_SECRET all the time. I also just always have AZURE_SUBCRIPTION_ID and AZURE_TENANT_ID set.

[marstr]

By the way, it's good to hear from you again @kpfaulkner!

[kpfaulkner]

Just setting up a new app for test purposes.

Yes, has been a while. Am liking the new storage lib/structure.... it does everything I need (so far) so have been quiet there. Now adding in a bunch of servicebus monitoring into our infrastructure, so now poking my nose into this repo :)

[devigned]

@kpfaulkner are you ok if we close this?

[kpfaulkner]

I dont have it working yet, I end up getting:

resources.GroupsClient#CreateOrUpdate: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: Service returned an error. Status=403 Code="AuthorizationFailed" Message="The client '3501f01e-14aa-4b54-ab95-a4391fe22eea' with object id '3501f01e-14aa-4b54-ab95-a4391fe22eea' does not have authorization to perform action 'Microsoft.Resources/subscriptions/resourcegroups/write' over scope '/subscriptions/2573fe04-9ee8-404a-aab4-6534c25b554e/resourcegroups/test'."

but assuming this is just me messing up the Azure application somehow.

Am good to close for this.. but might be a little while before I'm able to run the tests.

[devigned]

Well, it's progress 👍.

This illustrates a need for a developer getting started page, which would walk through getting a dev / test environment running.

The error you are running into right now is that the service principal you created doesn't have RBAC rights to the subscription to create a resource group. If you run this command, you should have an SP that will work for you az ad sp create-for-rbac.

On the other hand, you could provide the SP RBAC rights to your subscription via Manage Service Principal Roles.

Either route should get you over the current issue.

[devigned]

@kpfaulkner let's leave it open til you have executing tests. I think you are pretty close.

[kpfaulkner]

closer....

C:\Users\kenfa\projects\gopath\src\github.com\Azure\azure-service-bus-go>go test ./...
--- FAIL: TestSB (4.09s)
test.go:91:
Error Trace: test.go:91
suite.go:64
namespace_test.go:51
Error: Received unexpected error:
servicebus.NamespacesClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="UnsupportedResourceOperation" Message="The resource type 'namespaces' does not support this operation."
.
.
.
..

suggestions?

[kpfaulkner]

Just as a follow up, from what I can see the function func (client NamespacesClient) CreateOrUpdateSender(req *http.Request) (future NamespacesCreateOrUpdateFuture, err error) is the one that's blowing up on me.

When running, I can see the resource group "test" being made, then the following request:

PUT /subscriptions/2573fe04-9ee8-404a-aab4-6534c25b554e/resourceGroups/test/providers/Microsoft.ServiceBus/namespaces/?api-version=2017-04-01 HTTP/1.1

returns

{"error":{"code":"UnsupportedResourceOperation","message":"The resource type 'namespaces' does not support this operation."}}

[devigned]

Ahh... This is caused because the subscription you are using doesn't have the Microsoft.ServiceBus provider registered.

Background: Each Azure service is a "Resource Provider" those providers need to be registered in your subscription before they can be used.

The portal will do nice things like register the provider beforehand. The test code does not, but maybe, it should.

You can remedy this error by running az provider register -n 'Microsoft.ServiceBus'. If you want to know more about the command check out az provider

I hadn't had enough coffee when I wrote this. I believe the above is correct, but might not address the issue described.

[devigned]

@kpfaulkner it looks like the namespace name is empty when it tries to run the CreateOrUpdate here

azure-service-bus-go/internal/test/test.go

Lines 141 to 167 in 70612d6

	func (suite *BaseSuite) ensureProvisioned(tier sbmgmt.SkuTier) error {
	groupsClient := suite.getRmGroupClient()
	location := location
	_, err := groupsClient.CreateOrUpdate(context.Background(), resourceGroupName, rm.Group{Location: &location})
	if err != nil {
	return err
	}
	nsClient := suite.getNamespaceClient()
	_, err = nsClient.Get(context.Background(), resourceGroupName, suite.Namespace)
	if err != nil {
	ns := sbmgmt.SBNamespace{
	Sku: &sbmgmt.SBSku{
	Name: sbmgmt.SkuName(tier),
	Tier: tier,
	},
	Location: &location,
	}
	res, err := nsClient.CreateOrUpdate(context.Background(), resourceGroupName, suite.Namespace, ns)
	if err != nil {
	return err
	}
	return res.WaitForCompletion(context.Background(), nsClient.Client)
	}
	return nil

. I think that is the case because in the PUT, there is no /{name} after the /namespaces/ segment.

The namespace name is derived from the connection string via your environment var. See

azure-service-bus-go/internal/test/test.go

Lines 75 to 94 in 70612d6

	func (suite *BaseSuite) SetupSuite() {
	suite.TenantID = mustGetEnv("AZURE_TENANT_ID")
	suite.SubscriptionID = mustGetEnv("AZURE_SUBSCRIPTION_ID")
	suite.ClientID = mustGetEnv("AZURE_CLIENT_ID")
	suite.ClientSecret = mustGetEnv("AZURE_CLIENT_SECRET")
	suite.ConnStr = mustGetEnv("SERVICEBUS_CONNECTION_STRING")
	parsed, err := conn.ParsedConnectionFromStr(suite.ConnStr)
	if !suite.NoError(err) {
	suite.FailNow("connection string could not be parsed")
	}
	suite.Namespace = parsed.Namespace
	suite.Token = suite.servicePrincipalToken()
	suite.Environment = azure.PublicCloud
	suite.TagID = RandomString("tag", 10)
	suite.setupTracing()
	if !suite.NoError(suite.ensureProvisioned(sbmgmt.SkuTierStandard)) {
	suite.FailNow("failed to ensure provisioned")
	}
	}

Here's an example of what I mean. It also makes me think the connection parser should be a little more opinionated on what it thinks is an ok connection string. See the example below.

$ SERVICEBUS_CONNECTION_STRING="redacted=" go test -v
=== RUN   TestSB
--- FAIL: TestSB (2.73s)
	test.go:91:
			Error Trace:	test.go:91
			            				suite.go:64
			            				namespace_test.go:43
			Error:      	Received unexpected error:
			            	servicebus.NamespacesClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="UnsupportedResourceOperation" Message="The resource type 'namespaces' does not support this operation."
			Test:       	TestSB
	test.go:92:
			Error Trace:	test.go:92
			            				suite.go:64
			            				namespace_test.go:43
			Error:      	failed to ensure provisioned
			Test:       	TestSB
FAIL
exit status 1
FAIL	github.com/Azure/azure-service-bus-go	2.754s

[kpfaulkner]

Just an update:

Had quotes around my servicebus connection env var.... meant parsing died and didn't pick up name space.

Also, I had to modify test.go since there is an assumption about region and resourcegroup name.
Now tests run!!!!

Have modified PR #22 with a test confirming that after pushing a message the active message count is 1 (to at least confirm CountDetails is being populated). One thing I had to do and would like some advice on is I had to sleep between pushing message and retrieving queue (with CountDetails). Without the sleep it was fairly random (on my machine) if I'd get back my active message count or not (populated correctly). Although I notice another test also requires a sleep so maybe it's just a fact of life?

[devigned]

@kpfaulkner check out

azure-service-bus-go/queue_test.go

Lines 739 to 752 in 70612d6

	func waitUntil(t *testing.T, wg *sync.WaitGroup, d time.Duration) {
	done := make(chan struct{})
	go func() {
	wg.Wait()
	close(done)
	}()
	select {
	case <-done:
	return
	case <-time.After(d):
	t.Fatal("took longer than " + fmtDuration(d))
	}
	}

I experienced similar results and decided to give the active message count a chance to catch up based on the deadline of the test context. Each test has a default deadline and will wait for either the wait group to complete or the deadline to pass. This might be useful for you in this case.

[kpfaulkner]

I was wondering about that. But for the cases where waitUntil is already run we've got a handler parameter that we can incorporate the waitgroup completion into. In my case I'm simply calling Get(). If we want to avoid sleeps I can use a tight loop in a goroutine that keeps calling Get(), once it's completed mark the waitgroup as done. But that seems just as messy to me.

Is that what you were picturing? Or is there something I'm missing?

Cheers

[devigned]

Loop + sleep until success or deadline reached sounds good. No need to get too fancy.

[kpfaulkner]

Ok left it at that.

[devigned]

@kpfaulkner, I've merged the PR. I believe this solves your issue, so I'm going to close this. If you find this is still an issue, please reopen.