Skip to content

feat: handle clustered resource on secondary to primary mapper init #2313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 13 commits into from
Closed

feat: handle clustered resource on secondary to primary mapper init #2313

wants to merge 13 commits into from

Conversation

metacosm
Copy link
Collaborator

Fixes #2311

Signed-off-by: Chris Laprun [email protected]

csviri and others added 11 commits March 9, 2024 12:59
@csviri
improve: use fixed thread pool explicitly (#2265)
ab7c01f 

Signed-off-by: Attila Mészáros <[email protected]>
@dependabot
chore(deps): bump log4j.version from 2.23.0 to 2.23.1 (#2276)
15ff8b5 
Bumps `log4j.version` from 2.23.0 to 2.23.1.

Updates `org.apache.logging.log4j:log4j-slf4j-impl` from 2.23.0 to 2.23.1

Updates `org.apache.logging.log4j:log4j-core` from 2.23.0 to 2.23.1

Updates `org.apache.logging.log4j:log4j2-core` from 2.23.0 to 2.23.1

---
updated-dependencies:
- dependency-name: org.apache.logging.log4j:log4j-slf4j-impl
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: org.apache.logging.log4j:log4j-core
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: org.apache.logging.log4j:log4j2-core
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot
chore(deps): bump io.github.git-commit-id:git-commit-id-maven-plugin (#…
30cc18a 
…2277)

Bumps [io.github.git-commit-id:git-commit-id-maven-plugin](https://github.com/git-commit-id/git-commit-id-maven-plugin) from 8.0.0 to 8.0.1.
- [Release notes](https://github.com/git-commit-id/git-commit-id-maven-plugin/releases)
- [Commits](git-commit-id/git-commit-id-maven-plugin@v8.0.0...v8.0.1)

---
updated-dependencies:
- dependency-name: io.github.git-commit-id:git-commit-id-maven-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot
chore(deps): bump org.apache.maven.plugins:maven-gpg-plugin (#2282)
e4495f8 
Bumps [org.apache.maven.plugins:maven-gpg-plugin](https://github.com/apache/maven-gpg-plugin) from 3.1.0 to 3.2.0.
- [Release notes](https://github.com/apache/maven-gpg-plugin/releases)
- [Commits](apache/maven-gpg-plugin@maven-gpg-plugin-3.1.0...maven-gpg-plugin-3.2.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-gpg-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot
chore(deps): bump io.micrometer:micrometer-core from 1.12.3 to 1.12.4 (…
7b2d078 
…#2281)

Bumps [io.micrometer:micrometer-core](https://github.com/micrometer-metrics/micrometer) from 1.12.3 to 1.12.4.
- [Release notes](https://github.com/micrometer-metrics/micrometer/releases)
- [Commits](micrometer-metrics/micrometer@v1.12.3...v1.12.4)

---
updated-dependencies:
- dependency-name: io.micrometer:micrometer-core
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@csviri
improve: remote stale check (#2283)
0029b67 
This is already done by Openshift CI

Signed-off-by: Attila Mészáros <[email protected]>
@metacosm
fix: date parsing after 8.0.0 release of git-commit-id (#2286)
9674154 
Signed-off-by: Chris Laprun <[email protected]>
@actions-user
Set new SNAPSHOT version into pom files.
9fa2b68
@dependabot
chore(deps): bump org.apache.maven.plugins:maven-gpg-plugin (#2294)
4233483 
Bumps [org.apache.maven.plugins:maven-gpg-plugin](https://github.com/apache/maven-gpg-plugin) from 3.2.0 to 3.2.1.
- [Release notes](https://github.com/apache/maven-gpg-plugin/releases)
- [Commits](apache/maven-gpg-plugin@maven-gpg-plugin-3.2.0...maven-gpg-plugin-3.2.1)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-gpg-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot
chore(deps): bump io.github.git-commit-id:git-commit-id-maven-plugin (#…
0b17878 
…2295)

Bumps [io.github.git-commit-id:git-commit-id-maven-plugin](https://github.com/git-commit-id/git-commit-id-maven-plugin) from 8.0.1 to 8.0.2.
- [Release notes](https://github.com/git-commit-id/git-commit-id-maven-plugin/releases)
- [Commits](git-commit-id/git-commit-id-maven-plugin@v8.0.1...v8.0.2)

---
updated-dependencies:
- dependency-name: io.github.git-commit-id:git-commit-id-maven-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@shawkins
fix: ignore server managed fields for SSA matching (#2309)
43b8591 
Signed-off-by: Steven Hawkins <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
@metacosm metacosm self-assigned this Mar 25, 2024
@metacosm metacosm requested a review from csviri March 25, 2024 10:14
@openshift-ci openshift-ci bot requested a review from adam-sandor March 25, 2024 10:14
@metacosm metacosm force-pushed the handle-clustered-main branch from 0508a49 to 2509843 Compare March 25, 2024 10:16
csviri
csviri requested changes Mar 25, 2024
View reviewed changes
Copy link
Collaborator

@csviri csviri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@metacosm this is not good this way, it is the type of the secondary resource not the primary what you check. It should be the primary

@csviri csviri mentioned this pull request Mar 25, 2024
Closed
@metacosm metacosm force-pushed the handle-clustered-main branch from 2509843 to 89acf8d Compare March 25, 2024 14:56
@metacosm
fix: make primary type computation overridable
eab3bf0 
Overriding getPrimaryResourceType should allow to make things work even
in deeper hierarchies.

Signed-off-by: Chris Laprun <[email protected]>
@metacosm metacosm force-pushed the handle-clustered-main branch from fd63c25 to eab3bf0 Compare March 25, 2024 21:08
@metacosm
Copy link
Collaborator Author

I've changed the approach, but I'm not sure if the solution I chose is agreeable or not… I would say that it would work without changes most of the time but there are cases (as shown in some of the tests) where a light user intervention would be needed so maybe we want to:

  1. keep the change as-is and decide that it's ok for main
  2. keep the change as-is but schedule it for v5
  3. decide for a completely different approach altogether

Personally, I think that having the primary type available from the dependent resource might be useful (but with the current change, this is only available to descendants of KubernetesDependentResource).

@metacosm metacosm requested a review from csviri March 26, 2024 07:52
@csviri
Copy link
Collaborator

csviri commented Mar 26, 2024

IMO this should target next, we might want to do few iteration also reflecting on other issues.

Personally, I think that having the primary type available from the dependent resource might be useful (but with the current change, this is only available to descendants of KubernetesDependentResource).

Basically, we need primary just for this case for now, so having it as a configuration option (boolean) for KubernetesDependentResource would be enough.

Also, I think ATM is enough to have for Kubernetes Dependents. Only those use owner references.

@metacosm metacosm changed the base branch from main to next March 26, 2024 15:14
@metacosm
Copy link
Collaborator Author

OK, retargering to next.

@metacosm
Copy link
Collaborator Author

Closed in favor of #2321

@metacosm metacosm closed this Mar 26, 2024
@metacosm metacosm deleted the handle-clustered-main branch March 26, 2024 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adam-sandor adam-sandor Awaiting requested review from adam-sandor

@csviri csviri Awaiting requested review from csviri

Assignees

@metacosm metacosm

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Handling Cluster Scoped Primary Resource in Dependent Resource SecondaryToPrimaryMapper
4 participants
@metacosm @csviri @actions-user @shawkins