Skip to content
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

docs(proposal): support multiple replicas with leader election #5051

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

ivankatliarchuk
Copy link
Contributor

@ivankatliarchuk ivankatliarchuk commented Jan 31, 2025

Description

Full pull request is here #5033

Motivation for split #4598 (comment)

The plan is to add documentation, and move main logic from main.go to it's own function as part of this pull request. This should help to focus on testing an actual feature, without constant need to rebase main.go file. Currently all the flags changes affects main.go, this may need to be addressed at some point as well.

Checklist

  • Unit tests updated
  • End user documentation updated

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 31, 2025
@ivankatliarchuk
Copy link
Contributor Author

/kind documentation
/king feature
/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added kind/documentation Categorizes issue or PR as related to documentation. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. labels Jan 31, 2025
@ivankatliarchuk ivankatliarchuk changed the title feat(leader-election): support multiple replicas. pre-requisits feat(leader-election): support multiple replicas. documentation Jan 31, 2025
@mloiseleur
Copy link
Contributor

mloiseleur commented Jan 31, 2025

@ivankatliarchuk You need to link this doc in mkdocs.yml, like the other proposal, see here.

There is a typo in the diagram: "urrentLeader" instead of "CurrentLeader".

BTW, if you can do the schema with mermaid instead of a png, it's (way) easier to maintain it. See https://kubernetes.io/docs/contribute/style/diagram-guide/

@ivankatliarchuk
Copy link
Contributor Author

Addressed

Copy link
Contributor

@mloiseleur mloiseleur left a comment

Choose a reason for hiding this comment

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

Thanks, it's quite clear 👍 .
See my suggestion for minor improvements.

I'm unsure if you really wanted to add internal/testutils/logs.go to this PR ?

@ivankatliarchuk
Copy link
Contributor Author

Yeah, that logs.go could w8. Removed.

@mloiseleur
Copy link
Contributor

/retitle docs(proposal): support multiple replicas with leader election

@k8s-ci-robot k8s-ci-robot changed the title feat(leader-election): support multiple replicas. documentation docs(proposal): support multiple replicas with leader election Feb 2, 2025
@mloiseleur
Copy link
Contributor

/lgtm

@szuecs @Raffo @Dadeos-Menlo You are more than welcome to review this proposal 🙏

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 2, 2025
@mloiseleur
Copy link
Contributor

Part of #2430

@ivankatliarchuk
Copy link
Contributor Author

/kind proposal

@k8s-ci-robot
Copy link
Contributor

@ivankatliarchuk: The label(s) kind/proposal cannot be applied, because the repository doesn't have them.

In response to this:

/kind proposal

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Signed-off-by: ivan katliarchuk <[email protected]>
* master:
  docs: add deprecation policy (kubernetes-sigs#5053)
  style: space vs tabs
  fix: remove misleading error message
  fix(cloudflare): ignore 1002 permission denied errors in UpdateDataLocalizationRegionalHostname
@k8s-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 2, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from mloiseleur. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ivankatliarchuk
Copy link
Contributor Author

Added header

---
title: leader election proposal
version: v1
authors: 
creation-date: 
status: draft
---

@ivankatliarchuk
Copy link
Contributor Author

/kind feature

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Feb 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/documentation Categorizes issue or PR as related to documentation. kind/feature Categorizes issue or PR as related to a new feature. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants