Skip to content
This repository has been archived by the owner on May 16, 2023. It is now read-only.

[filebeat] variable maxUnavailable updateStrategy #809

Merged
merged 8 commits into from
Jan 28, 2021
Merged

[filebeat] variable maxUnavailable updateStrategy #809

merged 8 commits into from
Jan 28, 2021

Conversation

clemcvlcs
Copy link
Contributor

Problem

Unable to set a custom value for the MaxUnavalaible for the rollingUpdate filebeat's update strategy.
This option should be available especially for filbert, because there is no need to have a MaxUnavailable set to 1 when there's one filebeat pod per node.

What I've done in PR

Add an {{- if }} statement to the updateStrategy regarding the MaxUnavaillable variable. If the variable is not set, the default value is et to 1 (current value set right now).

Chart version

filebeat 7.9.1

Tools versions

Helm version
Client: {Version:"v3.3.0", GitCommit:"8a4aeec08d67a7b84472007529e8097ec3742105", GitTreeState:"dirty", GoVersion:"go1.14.6"}

kubectl version
Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.8", GitCommit:"9f2892aab98fe339f3bd70e3c470144299398ace", GitTreeState:"clean", BuildDate:"2020-08-13T16:12:48Z", GoVersion:"go1.13.15", Compiler:"gc", Platform:"darwin/amd64"}

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@cla-checker-service
Copy link

cla-checker-service bot commented Sep 14, 2020

💚 CLA has been signed

@clemcvlcs clemcvlcs closed this Sep 14, 2020
@clemcvlcs clemcvlcs reopened this Oct 6, 2020
@clemcvlcs
Copy link
Contributor Author

cla signed

@jmlrt jmlrt added the filebeat label Oct 7, 2020
@jmlrt
Copy link
Member

jmlrt commented Oct 14, 2020

jenkins test this please

Copy link
Member

@jmlrt jmlrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to fix typo and update readme file. the rest lgtm 👍

filebeat/values.yaml Outdated Show resolved Hide resolved
Copy link
Member

@jmlrt jmlrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to fix typo and update readme file. the rest lgtm 👍

@clemcvlcs
Copy link
Contributor Author

cla signed

@clemcvlcs clemcvlcs requested a review from jmlrt October 19, 2020 11:58
@jmlrt
Copy link
Member

jmlrt commented Oct 19, 2020

cla/check

@jmlrt
Copy link
Member

jmlrt commented Oct 26, 2020

LGTM⛴

@jmlrt jmlrt changed the title variable MaxUnavailbale updateStrategy [filebeat] variable MaxUnavailable updateStrategy Oct 26, 2020
Copy link
Member

@jmlrt jmlrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM⛴

@jmlrt
Copy link
Member

jmlrt commented Oct 27, 2020

jenkins test this please

Copy link
Member

@jmlrt jmlrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, MaxUnavailable value is still missing in the README value table. Can you add it please?

@botelastic
Copy link

botelastic bot commented Jan 25, 2021

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. To track this PR (even if closed), please open a corresponding issue if one does not already exist.

@jmlrt
Copy link
Member

jmlrt commented Jan 28, 2021

@clemcvlcs, I merged master on your branch and updated it in regard of #964 which is adding Filebeat deployment, also updated README and tests.

I can't push on your branch so my changes are 2ce2c79, 873fd83 and 7029852.

Are you interested to update your PR with these changes so we can merge it?

@botelastic botelastic bot removed the triage/stale label Jan 28, 2021
@clemcvlcs clemcvlcs requested a review from jmlrt January 28, 2021 09:44
@jmlrt
Copy link
Member

jmlrt commented Jan 28, 2021

jenkins test this please

@jmlrt
Copy link
Member

jmlrt commented Jan 28, 2021

Ooops forgot to format python tests https://devops-ci.elastic.co/job/elastic+helm-charts+pull-request+lint-python/784/label=docker&&virtual&&ubuntu-18.04/console 🤦

Can you run black filebeat/tests?

The other error (https://devops-ci.elastic.co/job/elastic+helm-charts+pull-request+template-testing/1163/CHART=filebeat,label=docker&&virtual/console) is strange and I don't have it locally. I'll rerun the CI tests after formatting.

@jmlrt
Copy link
Member

jmlrt commented Jan 28, 2021

jenkins test this please

Copy link
Member

@jmlrt jmlrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM⛴ Black tests are failing on some part of the code which haven't been changed 🤷‍♂️. I'll fix it in another PR if required.

@jmlrt jmlrt changed the title [filebeat] variable MaxUnavailable updateStrategy [filebeat] variable maxUnavailable updateStrategy Jan 28, 2021
@jmlrt jmlrt merged commit 9f28ee6 into elastic:master Jan 28, 2021
@jb-abbadie jb-abbadie deleted the maxUnavailbale_variable branch January 28, 2021 11:56
@jmlrt jmlrt mentioned this pull request Feb 8, 2021
This was referenced Feb 12, 2021
@jmlrt jmlrt mentioned this pull request Feb 18, 2021
This was referenced Mar 15, 2021
@jmlrt jmlrt mentioned this pull request May 25, 2021
@jmlrt jmlrt mentioned this pull request Mar 8, 2022
@jmlrt jmlrt mentioned this pull request Apr 21, 2022
This was referenced Sep 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants