-
Notifications
You must be signed in to change notification settings - Fork 10
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.
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
fix: replace kustomize with $(KUSTOMIZE) #224
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes a missing kustomize in dev-deploy related to #165 |
phoban01
approved these changes
Jun 23, 2023
19 tasks
Skarlso
added a commit
that referenced
this pull request
Oct 29, 2024
## Description Fix for issue [ #68](open-component-model/ocm-project#68) in ocm-project. At least as of 0.23.7 there was primary problem and a follow on problem that appeared after the first was fixed. My setup was like this ![image](https://github.com/user-attachments/assets/c6015cee-5c88-4644-915e-852e0b94bc15) With the setup and v0.23.7 the first problem was I would get an error like this `ocm-system dmi-broker 24m False artifact revision 19090 does not match chart version 1.3.0-1085.xeae3fba.14` This because snapshot writer is using the resourceversion of of the kubernetes object that owns the snapshot. Given my setup, 19090 happened to be the resourceversion of my Configuration. As I mentioned when I opened [#68](open-component-model/ocm-project#68), at least for helmcharts this is silly as there are several spots that are better sources for a version tag. So I have fixed this storying the resource version, taken from the component descriptor, in the identity. I am only doing this if there isn't already a value stored under ResourceVersionKey. Fixing this revealed a problem when you have more than 1 mutation, in my case a Localization and a Configuration, getting their config a resource. The mutation objects get their identity from the resource they reference in `configRef`. Both of my reference the same config.yaml in my components. The identity is used to compute the snapshot's OCI repository name. Prior to my above tag fix being in place, the Localization and Configuration were both sticking their snapshots into the same repository. Since I was 'lucky' and the two objects never had the same resourceversion the tags were used on the images stored in the repository were different and I never noticed a problem. However once I put the above fix in place I started seeing sporadic failures. In fact, due to issue #224, I would see my HelmRelease go back and forth between being unhealthy and healthy ( but mostly unhealthy ). It would be unhealthy if/when the FluxDeployer grabbed the data from the snapshot image when the Localization was the last one to write the snapshot image. To fix this added I 'mutation-object-uuid' to the identity of resources that are output from the mutations ( Localization, Configuration ). This way each will have its own snapshot repo and they will no longer overwrite each other's images when the same resource is referenced in `configRef`. ## What type of PR is this? (check all applicable) - [ ] 🍕 Feature - [x ] 🐛 Bug Fix - [ ] 📝 Documentation Update - [ ] 🎨 Style - [ ] 🧑💻 Code Refactor - [ ] 🔥 Performance Improvements - [ ] ✅ Test - [ ] 🤖 Build - [ ] 🔁 CI - [ ] 📦 Chore (Release) - [ ] ⏩ Revert ## Related Tickets & Documents <!-- Please use this format link issue numbers: Fixes #123 https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword --> - Fixes [68](open-component-model/ocm-project#68) ## Added tests? - [ ] 👍 yes - [ ] 🙅 no, because they aren't needed - [ ] 🙋 no, because I need help - [ ] Separate ticket for tests # (issue/pr) As of 2024-07-11 I have just used the fixed build in my POC and confirmed that my components are being deployed. If/when I get time I'll try to update the test suite. ## Added to documentation? - [ ] 📜 README.md - [ ] 🙅 no documentation needed ## Checklist: - [x ] My code follows the style guidelines of this project - [ x] I have performed a self-review of my code - [ x] I have commented my code, particularly in hard-to-understand areas - [ x] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ x] Any dependent changes have been merged and published in downstream modules --------- Co-authored-by: Gergely Brautigam <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.