-
Notifications
You must be signed in to change notification settings - Fork 78
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
add toggle features for Kafka and minor fixes around operator plan #239
Conversation
Signed-off-by: Zain Malik <zmalikshxil@gmail.com>
880fa50
to
0ce3e00
Compare
- health-check.yaml | ||
- enable-tls.yaml | ||
- user-workload.yaml | ||
# task used to restrict patching sts where it is not allowed | ||
- name: not-allowed | ||
kind: Dummy |
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.
On the topic of the not-allowed
task: Would it make sense to have a task type "Fail" that simply FATAL_ERRORs the plan with a specified message?
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.
yeah we need to do another round here now after many releases
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.
Actually, I think we really need immutable
parameters. Having the task is ok-ish for now, but the parameters still get set even if the plan fails. And on the next run of update
they're applied
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.
And our admission controller would be able to prevent changing immutable parameters. We need a KEP for that but otherwise, the solution can be pretty simple.
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.
Looks good to me. I'm probably not grasping all the details, but I couldn't see any obvious issues
Signed-off-by: Zain Malik <zmalikshxil@gmail.com>
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.
Nice work! 🚢
this PR:
README.md
for unreleased versions to avoid any confusiondeploy
andupdate
plans, to avoid running pipe task in each update. This should also reduce the test duration in all CI pipelinesapps/v1
$root
as Add verifier support for the special$
variable. kudo#1255 was merged and released