feat: add revisionHistoryLimit

[KyriosGN0]

Type of change

• Enhancement / new feature

Description

adding the ability to set the revisionHistoryLimit for the operator Deployment

Checklist

Please go through this checklist and make sure all applicable tasks have been done

• Write tests
• Make sure all tests pass
• Update documentation
• Check RBAC rights for Kubernetes / OpenShift roles
• Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
• Reference relevant issue(s) and close them after merging
• Update CHANGELOG.md
• Supply screenshots for visual changes, such as Grafana dashboards

[scholzj]

Thanks for the PR. I left one nit. I also suspect that the build will fail. You have to:

• Either upgrade the YAML installation files correspondingly to this change
• Wrap the Helm chart change with some condition to not set it when not configured by the user. (see how it is done for the other fields such as the priorityClassName as example)

I personally would prefer the second option.

[scholzj]

Please don't change this.

[KyriosGN0]

thanks @scholzj, i opted to just set a default value in the yaml file

[scholzj]

thanks @scholzj, i opted to just set a default value in the yaml file

That does not fix it because it will still generate the YAML with the value You would need to do it as it is done for example for the priority class name: https://github.com/strimzi/strimzi-kafka-operator/blob/main/packaging/helm-charts/helm3/strimzi-kafka-operator/templates/060-Deployment-strimzi-cluster-operator.yaml#L45-L47

[KyriosGN0]

thanks @scholzj, i opted to just set a default value in the yaml file

That does not fix it because it will still generate the YAML with the value You would need to do it as it is done for example for the priority class name: https://github.com/strimzi/strimzi-kafka-operator/blob/main/packaging/helm-charts/helm3/strimzi-kafka-operator/templates/060-Deployment-strimzi-cluster-operator.yaml#L45-L47

@scholzj fixed that, but i would like to understand where is the issue?
if a user would create a values file without this param it would have used the default..
or am i missing something?

[KyriosGN0]

hey @scholzj @ppatierno could you help me figure out what went wrong in the CI, it complains about uncommitted changes in a dir a didn't touch?
am i missing something?

[scholzj]

hey @scholzj @ppatierno could you help me figure out what went wrong in the CI, it complains about uncommitted changes in a dir a didn't touch? am i missing something?

Yeah, this is the error I talked about. I think the Deployment YAMl in the Helm Chart looks good now. But you have to also remove the default value from the values file to not have it rendered into the YAML.

It also looks like some older commits got pulled into your PR in some rebase, could you please try to fix it as well?

[scholzj]

/azp run helm-acceptance

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[scholzj]

LGTM. Thanks.

[ppatierno]

LGTM. Thanks!