-
Notifications
You must be signed in to change notification settings - Fork 205
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Updates TektonInstallerSet to allow replica changes to a deployment #530
Conversation
The following is the coverage report on the affected files.
|
/hold |
holding this pr so that we can make sure that we all agree on the level of control the operator should have on resources managed by operator. |
@sm43 could you add the additional check where the exptetedhash is compared to annotation hash along with the exiting check. |
This now updates the installer set to allow replica changes, it does it as follow - when creating a deployment it compute hash of spec and add it as annotation - while computing hash it set replicas to a const so that next time we can set replicas to same const and then calculate hash - if hash is different which would mean something is changed in spec other than hash - so it revert the spec to orignal and updates the replicas to what was on the cluster deployment - it will allow replica change but revert any other change to spec as the hash would change Signed-off-by: Shivam Mukhade <smukhade@redhat.com>
fa77d68
to
89fa84b
Compare
The following is the coverage report on the affected files.
|
/hold cancel |
/lgtm |
@sm43 should we add unit test to cover pending scenario so that coverage increases |
/lgtm |
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.
We will probably want to backport this to 0.50, 0.51 and 0.52 ? 😛
/hold |
@savitaashture I will address the tests in a separate pr. :) we can merge this. |
Sure 👍 |
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.
🎉
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: savitaashture, vdemeester The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This now updates the installer set to allow replica changes, it does
it as follow
annotation
can set replicas to same const and then calculate hash
other than hash
on the cluster deployment
hash would change
Closes #491
Signed-off-by: Shivam Mukhade smukhade@redhat.com
/cc @nikhil-thomas @vdemeester @savitaashture
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Release Notes