-
Notifications
You must be signed in to change notification settings - Fork 11
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
CR-5279 - As a user I'll be able to upgrade runtime to a specific version #19
Conversation
* persist runtime.yaml in repo (inside a configmap) * updated argo-events to v1.4.0 * added managed-by to eventSource and sensor
Codecov Report
@@ Coverage Diff @@
## main #19 +/- ##
=======================================
Coverage 25.13% 25.13%
=======================================
Files 1 1
Lines 187 187
=======================================
Hits 47 47
Misses 134 134
Partials 6 6 Continue to review full report at Codecov.
|
} | ||
|
||
if urlObj.Query().Get("ref") == "" { | ||
urlObj.Query().Set("ref", version.String()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should check wether version was set, it can be nil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the user doesn't supply a version, the command puts in the current binary version as the version. So when we get to here, it will always contain a valid value.
} | ||
|
||
func (r *Runtime) Upgrade(fs fs.FS, newRt *Runtime) ([]AppDef, error) { | ||
newComponents, err := r.Spec.upgrade(fs, &newRt.Spec) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why upgrade only return the components to be created but do the actual delete ? I suggest that it will also do the creation/update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a new app is a bit more complicated than just deleting it. So I preferred leaving it to autopilot.
In the autopilot, we always do clone->change->commit->push, so I had to call all the RunAppCreate
after the commit/push of the rest of the changes (update and delete of applications)
I think we should refactor Autopilot to be able to get an existing filesystem, with an already existing repo cloned to it, and then just make the changes, without commit/push at the end. I already made a plan on how to implement it. It will make a lot of our cli-v2 code much simpler.
No description provided.