Skip to content
This repository has been archived by the owner on Dec 6, 2024. It is now read-only.

Fix recording events #85

Merged
merged 1 commit into from
Aug 1, 2024
Merged

Conversation

kvaps
Copy link

@kvaps kvaps commented Aug 1, 2024

There is an issue with writing events in different namespaces: kubernetes-retired/container-object-storage-interface-provisioner-sidecar#140

This happens because events are created with a specified namespace, and the EventRecorder also has a namespace option set.

This PR removes the namespace option from the EventRecorder so events can be created in namespace they have specifyed.

Similar change can be found in this project
tsuru/remesher@a8ccfa0#diff-243ebed2765f75e6a54f57167212fefb08c3b2a85967ad2acbc0eb78919019c1

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 1, 2024
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 1, 2024
@BlaineEXE
Copy link
Contributor

BlaineEXE commented Aug 1, 2024

@kvaps could you please add more description to the commit and github description? What are the symptoms of the issue, and how does this PR fix the issue?

[update] I see after looking at your other PR that this is a fix for kubernetes-retired/container-object-storage-interface-provisioner-sidecar#140.

@BlaineEXE
Copy link
Contributor

@kvaps we really appreciate your contribution. There is a complication that makes this challenging right now. We are in the process of migrating from separate API/spec/controller/sidecar repos into a monorepo at https://github.com/kubernetes-sigs/container-object-storage-interface-api. We are still working on merging the controller into the monorepo, and the sidecar will come after that.

In the interest of ensuring your contribution gets recorded, I will propose this:

Could you change the merge target from kubernetes-sigs:master to kubernetes-sigs:monrepo (the monorepo branch)? We can then merge this, and the inclusion of the change will be handled once we finish moving the controller/controller.go code into its new location where it will be a better-named basis for shared code between controller and sidecar.

@kvaps kvaps changed the base branch from master to monorepo August 1, 2024 17:10
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 1, 2024
Signed-off-by: Andrei Kvapil <[email protected]>
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 1, 2024
@kvaps
Copy link
Author

kvaps commented Aug 1, 2024

sure PR rebased

@BlaineEXE
Copy link
Contributor

Also requesting @shanduur 's review since he worked on adding event recording

@shanduur
Copy link
Contributor

shanduur commented Aug 1, 2024

Ah, right! Must have missed that bit, thanks!

@shanduur
Copy link
Contributor

shanduur commented Aug 1, 2024

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kvaps, shanduur

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 1, 2024
@BlaineEXE
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 1, 2024
@k8s-ci-robot k8s-ci-robot merged commit a56c8d1 into kubernetes-retired:monorepo Aug 1, 2024
4 checks passed
shanduur pushed a commit to shanduur/container-object-storage-interface-api that referenced this pull request Aug 2, 2024
…eate-multiarch-builder-for-use

set up cloudbuild multi-arch builder before use
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants