Bug 1827075: Drain GCS buckets before removal #554

adambkaplan · 2020-05-15T12:58:45Z

When removing GCS storage, ensure that there are no objects in the bucket prior to deletion.
Otherwise, removing the storage will fail with 409 CONFLICT errors.

openshift-ci-robot · 2020-05-15T12:58:50Z

@adambkaplan: This pull request references Bugzilla bug 1827075, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.5.0) matches configured target release for branch (4.5.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1827075: Drain GCS buckets before removal

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

adambkaplan · 2020-05-15T13:03:44Z

/assign @dmage

/cc @ricardomaraschini @coreydaley

/hold

I'd like to discuss how we can effectively test this. As @coreydaley knows, the GCS SDK does not have good test doubles that allow us to easily unit test. My thought is that I should add a TestRemoveStorage suite that does the following:

Sets up the image registry with the defaults for the platform we are running on
Create a namespace
Imports the UBI 7 and 8 imagestreams with pullthrough into the namespace
Remove the image registry - thereby removing the storage.

Do we already have something like this?

adambkaplan · 2020-05-19T13:49:31Z

/hold cancel

Added e2e test

adambkaplan · 2020-05-19T15:26:10Z

@dmage @ricardomaraschini ptal - added an e2e test to ensure the image registry can be removed after images had been imported to it.

adambkaplan · 2020-05-19T15:33:30Z

@dmage @ricardomaraschini ptal - I added an e2e to ensure we can tear down the registry if images were added to the storage (via pullthrough in this case).

dmage · 2020-05-19T16:14:23Z

@adambkaplan you don't use pull-through there. Image API doesn't interact with the registry.

Pushing a blob or using builds might be easier rather than using pull-through. Maybe it's better to add a Disruptive test to openshift-tests?

adambkaplan · 2020-05-19T16:35:16Z

using builds might be easier rather than using pull-through

I can give that a go here - you can kick off a build using raw Build objects (ci-operator does this today)

When removing GCS storage, ensure that there are no objects in the bucket prior to deletion. Otherwise, removing the storage will fail with 409 CONFLICT errors. Added e2e test to ensure the image registry can be removed if an image had been pushed to it. Image push is accomplished via an OpenShift build, using our canonical NodeJS example. Test framework code updated to allow logs to be dumped from a single pod.

Vendor in image and build API code from openshift/api and client-go referenced in test code. This does not bump dependencies - it merely vendors in code from existing modules.

adambkaplan · 2020-05-20T14:34:09Z

@dmage updated to use a build, and the specific test had clean runs on GCP and AWS. PTAL

adambkaplan · 2020-05-20T16:38:11Z

/retest

Flake (?) on TestOperandProxyConfiguration

dmage · 2020-05-21T19:17:14Z

/lgtm

openshift-ci-robot · 2020-05-21T19:17:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adambkaplan, dmage

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [adambkaplan,dmage]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2020-05-21T19:37:33Z

/retest