search

cisco-open / cluster-registry-controller

An operator that automatically synchronizes Kubernetes resources across multiple clusters
Apache License 2.0
22 stars 8 forks source link

Allow mutate target ns #51

Closed BEvgeniyS closed 1 year ago

BEvgeniyS commented 1 year ago
Q A
Bug fix? maybe
New feature? maybe
API breaks? no
Deprecations? no
Related tickets Hopefully fixes https://github.com/cisco-open/cluster-registry-controller/issues/50
License Apache 2.0

What's in this PR?

This change

Why?

Currently, if we mutate object's namespace, it gets created and immediately deleted. See more in linked issue

Checklist

BEvgeniyS commented 1 year ago

CC: @pregnor @waynz0r @Laci21 @tiswanso

waynz0r commented 1 year ago

Hi @BEvgeniyS,

thanks for the PR!

It looks fine at first glance, but some internal testing seems adequate as namespace mutation could cause tricky side effects.

BEvgeniyS commented 1 year ago

@waynz0r @pregnor Thanks for the review. When you say internal testing, you mean you'd like to test it yourself or that you'd like to see some unit tests for this change?

waynz0r commented 1 year ago

When you say internal testing, you mean you'd like to test it yourself or that you'd like to see some unit tests for this change?

I meant to test it ourselves and I just did.