Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

Add tests for two more Storage resource types, and fixes to support #320

Merged
merged 16 commits into from
Nov 12, 2020

Conversation

Porges
Copy link
Member

@Porges Porges commented Nov 10, 2020

  • Add create/delete tests for the StorageAccountsBlobService and StorageAccountsBlobServicesContainer resource types.
  • Attempt to make deployment more robust, if we deploy a second time and the deployment already exists. This could happen due to faults or spurious failures in patching resources.
    • TODO: We need to check the existing deployment against spec and delete/recreate it if it is different?
  • Handle authentication configuration errors as early as possible; the autorest library does not check this and will give back an authorizer that does not work.
  • Added temporary fix for #319.
  • Fix bad log message in one of the matchers.
  • Only save recording for a test when it is successful. This makes it easier to develop when trying to fix a test.

Outstanding items

We need to think about how to handle resources like StorageAccountsBlobService better. It must be named "default" to work but this is not enforced at the moment.

One idea is that we could merge this into the StorageAccount resource type so that it can be configured there, and the controller takes care of creating it separately.

Another issue is that the name is not going to be unique in Kubernetes. For resources with an owner we might need to scope the name under the owner name, but I’m not sure that this is possible; the object is created in Kubernetes before we can touch the name. The user can set AzureName to "default" and Name to something else but this is not obvious.

I opened an issue for this here: #324

@Porges Porges changed the title Add tests for two more storage types, and fixes to support Add tests for two more storage resource types, and fixes to support Nov 10, 2020
@coveralls
Copy link

coveralls commented Nov 10, 2020

Coverage Status

Coverage increased (+16.6%) to 63.73% when pulling 19e46d9 on more-tests into e2be304 on master.

hack/generated/controllers/controller_test.go Outdated Show resolved Hide resolved
hack/generated/controllers/controller_test.go Show resolved Hide resolved
hack/generated/controllers/controller_test.go Show resolved Hide resolved
hack/generated/controllers/generic_controller.go Outdated Show resolved Hide resolved
hack/generated/pkg/armclient/raw_client.go Outdated Show resolved Hide resolved
@matthchr
Copy link
Member

Consider adding samples for the two new resource types in addition to tests?

@Porges Porges changed the title Add tests for two more storage resource types, and fixes to support Add tests for two more Storage resource types, and fixes to support Nov 11, 2020
@Porges Porges merged commit 3641fcf into master Nov 12, 2020
@Porges Porges deleted the more-tests branch November 12, 2020 19:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants