Skip to content

Bug 1954095: Setting user provided tags on bucket creation#679

Merged
openshift-merge-robot merged 5 commits intoopenshift:masterfrom
ricardomaraschini:aws-user-provided-tags-2
Apr 29, 2021
Merged

Bug 1954095: Setting user provided tags on bucket creation#679
openshift-merge-robot merged 5 commits intoopenshift:masterfrom
ricardomaraschini:aws-user-provided-tags-2

Conversation

@ricardomaraschini
Copy link
Contributor

OpenShift now supports users providing custom AWS tags to be set in all created objects. This PR sets users provided tags, as per enhancement proposal, only when the bucket is created by the operator. Again, as per enhancement proposal, already existing buckets are not updated if user changes tags at Infrastructure level.

Enhancement proposal: openshift/enhancements#706
API changes: openshift/api#864

OpenShift now supports users providing custom AWS tags to be set in all
created objects. This PR sets users provided tags, as per enhancement
proposal, only when the bucket is created by the operator.

Again, as per enhancement proposal, already existing buckets are not
updated if user changes tags at Infrastructure level.
Adds an unit test to check s3 driver is creating user provided tags when
creating a bucket.
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 16, 2021
@ricardomaraschini ricardomaraschini changed the title Setting user provided tags on bucket creation IR-188: Setting user provided tags on bucket creation Apr 16, 2021
We have two properties that implement Cronjobs(): V1 and V1beta1. As we
have been using v1beta1 across our codebase this PR silences a lint
complain by being specific about which function we want to call.
@ricardomaraschini
Copy link
Contributor Author

/retest

4 similar comments
@ricardomaraschini
Copy link
Contributor Author

/retest

@ricardomaraschini
Copy link
Contributor Author

/retest

@ricardomaraschini
Copy link
Contributor Author

/retest

@ricardomaraschini
Copy link
Contributor Author

/retest

@ricardomaraschini
Copy link
Contributor Author

/assign @dmage

@dmage
Copy link
Contributor

dmage commented Apr 21, 2021

/hold
/lgtm
/assign @wzheng1 @bmcelvee @sferich888

This is a new feature, so it should get xxx-approved before we can merge it.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 21, 2021
@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 21, 2021
@wzheng1
Copy link

wzheng1 commented Apr 22, 2021

@ricardomaraschini Could you please review test case created for this feature: https://url.corp.redhat.com/IR-188 ? Thanks!
cc @dmage

@gregsheremeta
Copy link

all of the acks are already on the epic, https://issues.redhat.com/browse/IR-187. Do we really need to hold this?

@ricardomaraschini
Copy link
Contributor Author

@wzheng1 LGTM

@bmcelvee
Copy link

bmcelvee commented Apr 22, 2021

/label docs-approved

Note no-doc: comment in https://issues.redhat.com/browse/IR-187.

@dmage
Copy link
Contributor

dmage commented Apr 27, 2021

hm...
/label docs-approved

@openshift-ci-robot openshift-ci-robot added the docs-approved Signifies that Docs has signed off on this PR label Apr 27, 2021
@sferich888
Copy link
Contributor

/label px-approved
/unassign @sferich888

@openshift-ci-robot openshift-ci-robot added the px-approved Signifies that Product Support has signed off on this PR label Apr 27, 2021
@dmage
Copy link
Contributor

dmage commented Apr 27, 2021

/title Bug 1954095: Setting user provided tags on bucket creation

@dmage
Copy link
Contributor

dmage commented Apr 27, 2021

/retitle Bug 1954095: Setting user provided tags on bucket creation

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Apr 28, 2021
@ricardomaraschini
Copy link
Contributor Author

/retest

1 similar comment
@ricardomaraschini
Copy link
Contributor Author

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 28, 2021

@ricardomaraschini: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-gcp c32b1ab link /test e2e-gcp

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@xiuwang
Copy link

xiuwang commented Apr 29, 2021

/label qe-approved
Validate the custom tags are created in S3 bucket.

@openshift-ci-robot openshift-ci-robot added the qe-approved Signifies that QE has signed off on this PR label Apr 29, 2021
@dmage
Copy link
Contributor

dmage commented Apr 29, 2021

/retitle Bug 1954095: Setting user provided tags on bucket creation

@openshift-ci-robot openshift-ci-robot changed the title IR-188: Setting user provided tags on bucket creation Bug 1954095: Setting user provided tags on bucket creation Apr 29, 2021
@openshift-ci-robot openshift-ci-robot added the bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. label Apr 29, 2021
@openshift-ci-robot
Copy link
Contributor

@ricardomaraschini: This pull request references Bugzilla bug 1954095, 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.8.0) matches configured target release for branch (4.8.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @wzheng1

Details

In response to this:

Bug 1954095: Setting user provided tags on bucket creation

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.

@openshift-ci-robot openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Apr 29, 2021
@dmage
Copy link
Contributor

dmage commented Apr 29, 2021

/lgtm
/hold cancel

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Apr 29, 2021
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [dmage,ricardomaraschini]

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

@dmage
Copy link
Contributor

dmage commented Apr 29, 2021

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit b86b056 into openshift:master Apr 29, 2021
@openshift-ci-robot
Copy link
Contributor

@ricardomaraschini: All pull requests linked via external trackers have merged:

Bugzilla bug 1954095 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1954095: Setting user provided tags on bucket creation

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.

@ricardomaraschini
Copy link
Contributor Author

/cherrypick release-4.7

@openshift-cherrypick-robot

@ricardomaraschini: #679 failed to apply on top of branch "release-4.7":

Applying: Bumping openshift/api
.git/rebase-apply/patch:9849: trailing whitespace.
          codeCoverageTool: Cobertura 
.git/rebase-apply/patch:40833: trailing whitespace.
	
.git/rebase-apply/patch:98305: space before tab in indent.
    	var node = nodes[i];
.git/rebase-apply/patch:98306: space before tab in indent.
    	var name = node.id.toString();
.git/rebase-apply/patch:98307: space before tab in indent.
    	var block = name.substring(name.lastIndexOf("_")+1);
warning: squelched 25 whitespace errors
warning: 30 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	go.mod
M	go.sum
M	vendor/modules.txt
Falling back to patching base and 3-way merge...
Auto-merging vendor/modules.txt
Removing vendor/k8s.io/component-base/version/def.bzl
Removing vendor/k8s.io/client-go/pkg/version/def.bzl
Removing vendor/k8s.io/client-go/listers/settings/v1alpha1/podpreset.go
Removing vendor/k8s.io/client-go/kubernetes/typed/settings/v1alpha1/podpreset.go
Removing vendor/k8s.io/client-go/kubernetes/typed/settings/v1alpha1/fake/fake_podpreset.go
Removing vendor/k8s.io/apiserver/pkg/registry/rest/export.go
Removing vendor/k8s.io/api/settings/v1alpha1/types_swagger_doc_generated.go
Removing vendor/k8s.io/api/settings/v1alpha1/types.go
Removing vendor/k8s.io/api/settings/v1alpha1/generated.proto
Removing vendor/k8s.io/api/batch/v2alpha1/types_swagger_doc_generated.go
Removing vendor/k8s.io/api/batch/v2alpha1/types.go
Removing vendor/k8s.io/api/batch/v2alpha1/generated.proto
Removing vendor/honnef.co/go/tools/staticcheck/vrp/vrp.go
Removing vendor/honnef.co/go/tools/staticcheck/vrp/string.go
Removing vendor/honnef.co/go/tools/staticcheck/vrp/slice.go
Removing vendor/honnef.co/go/tools/staticcheck/vrp/int.go
Removing vendor/honnef.co/go/tools/staticcheck/vrp/channel.go
Removing vendor/honnef.co/go/tools/staticcheck/knowledge.go
Removing vendor/honnef.co/go/tools/staticcheck/CONTRIBUTING.md
Removing vendor/honnef.co/go/tools/ssautil/ssautil.go
Removing vendor/honnef.co/go/tools/ssa/write.go
Removing vendor/honnef.co/go/tools/ssa/testmain.go
Removing vendor/honnef.co/go/tools/ssa/lift.go
Removing vendor/honnef.co/go/tools/simple/CONTRIBUTING.md
Removing vendor/honnef.co/go/tools/internal/passes/buildssa/buildssa.go
Removing vendor/honnef.co/go/tools/functions/pure.go
Removing vendor/google.golang.org/protobuf/internal/genname/name.go
Removing vendor/google.golang.org/protobuf/internal/fieldnum/wrappers_gen.go
Removing vendor/google.golang.org/protobuf/internal/fieldnum/type_gen.go
Removing vendor/google.golang.org/protobuf/internal/fieldnum/timestamp_gen.go
Removing vendor/google.golang.org/protobuf/internal/fieldnum/struct_gen.go
Removing vendor/google.golang.org/protobuf/internal/fieldnum/source_context_gen.go
Removing vendor/google.golang.org/protobuf/internal/fieldnum/field_mask_gen.go
Removing vendor/google.golang.org/protobuf/internal/fieldnum/empty_gen.go
Removing vendor/google.golang.org/protobuf/internal/fieldnum/duration_gen.go
Removing vendor/google.golang.org/protobuf/internal/fieldnum/doc.go
Removing vendor/google.golang.org/protobuf/internal/fieldnum/descriptor_gen.go
Removing vendor/google.golang.org/protobuf/internal/fieldnum/api_gen.go
Removing vendor/google.golang.org/protobuf/internal/fieldnum/any_gen.go
Removing vendor/google.golang.org/api/internal/pool.go
Removing vendor/golang.org/x/exp/cmd/apidiff/main.go
Removing vendor/golang.org/x/exp/apidiff/report.go
Removing vendor/golang.org/x/exp/apidiff/messageset.go
Removing vendor/golang.org/x/exp/apidiff/correspondence.go
Removing vendor/golang.org/x/exp/apidiff/compatibility.go
Removing vendor/golang.org/x/exp/apidiff/apidiff.go
Removing vendor/golang.org/x/exp/apidiff/README.md
Removing vendor/github.com/prometheus/procfs/cpuinfo_ppc64le.go
Removing vendor/github.com/prometheus/procfs/cpuinfo_mipsle.go
Removing vendor/github.com/prometheus/procfs/cpuinfo_mips64le.go
Removing vendor/github.com/prometheus/procfs/cpuinfo_arm64.go
Removing vendor/github.com/konsorten/go-windows-terminal-sequences/sequences_dummy.go
Removing vendor/github.com/konsorten/go-windows-terminal-sequences/sequences.go
Removing vendor/github.com/konsorten/go-windows-terminal-sequences/go.mod
Removing vendor/github.com/konsorten/go-windows-terminal-sequences/README.md
Removing vendor/github.com/konsorten/go-windows-terminal-sequences/LICENSE
Removing vendor/github.com/golang/protobuf/protoc-gen-go/descriptor/descriptor.pb.go
Removing vendor/cloud.google.com/go/storage/.repo-metadata.json
Removing vendor/cloud.google.com/go/issue_template.md
Removing vendor/cloud.google.com/go/iam/.repo-metadata.json
Removing vendor/cloud.google.com/go/compute/metadata/.repo-metadata.json
Auto-merging go.sum
CONFLICT (content): Merge conflict in go.sum
Auto-merging go.mod
CONFLICT (content): Merge conflict in go.mod
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Bumping openshift/api
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick release-4.7

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. docs-approved Signifies that Docs has signed off on this PR lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.