Remove builder cache pvc #1225
Merged
Remove builder cache pvc #1225
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
Co-authored-by: Yannick Perrenet <26223174+yannickperrenet@users.noreply.github.com>
Comment on lines
+82
to
+83
| // Deprecated and ignored. TODO: remove it? | ||
| ConfigDirVolumeSize string `json:"configDirVolumeSize,omitempty"` |
There was a problem hiding this comment.
@nhaghighat How would we go about deprecating fields? As far as I understood, deleting a field is not possible in a backwards compatible way?
There was a problem hiding this comment.
I've actually removed BuilderCacheDirVolumeSize and it worked, what ended up happening is that, if the entry existed in the CRD, the entry got removed. I guess the reason is that this way one can update a CRD definition seamlessly (?).
There was a problem hiding this comment.
Hmmm so you did something along the lines of:
- Orchest is installed with
BuilderCacheDirVolumeSizein theOrchestClusterCR Object. - Made these code changes where
BuilderCacheDirVolumeSizewas removed - Updated Orchest so that the new
orchest-controlleris used and new CRD is applied - The
OrchestClusterdefinition actually no longer listsBuilderCacheDirVolumeSizeand Orchest continues to run smoothly
Pretty cool 🤔 (so a new CRD doesn't break existing CR Objects if an item is deleted).
There was a problem hiding this comment.
Yeah, that's what I've experienced
yannickperrenet
approved these changes
Aug 24, 2022
Co-authored-by: Yannick Perrenet <26223174+yannickperrenet@users.noreply.github.com>
Merged
fruttasecca
merged commit 8586f2b
into
improv/remove-periodic-builder-cache-cleanup-task
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.
Description
Removes the builder cache PVC since it won't be used anymore after the changes to how we build environments.
I've tested both an install and an update, updates have been tested both with a
builder-cache-pvc-sizespecified on install (which leads to the entry being in the CRD) and without.Checklist