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

Feat/allow replica of 1 for Jetstream #2822

Merged
merged 40 commits into from
Oct 5, 2023

Conversation

joelcomp1
Copy link
Contributor

Closes #2743

Tested using the following EventBus Config:

yaml spec: jetstream: imagePullSecrets: - name: private-registry replicas: 1 version: 2.9.19

Using Argo Events 1.8.1 with these changes. Then in my Argo Event helm chart I updated the stream config:

## Event bus configuration configs: ## JetStream event bus jetstream: streamConfig: # -- Number of replicas, defaults to 3 and requires minimal 3 replicas: 1

Need to fix issue with the nats.conf config map when replica of 3 is used

jmillage and others added 30 commits September 26, 2023 06:12
…tead

Signed-off-by: jmillage <jmillage@bcubed-corp.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…goproj#2774)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
Signed-off-by: GitHub <noreply@github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…proj#2780)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…goproj#2781)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
Signed-off-by: Ali Koyuncu <ali.koyuncu@iceye.fi>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…goproj#2790)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…#2789)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…roj#2787)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…rgoproj#2788)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…proj#2791)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…roj#2797)

Signed-off-by: Liam Wyllie <risset@mailbox.org>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…roj#2795)

Signed-off-by: gokulav137 <gokulav999@gmail.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…goproj#2803)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…proj#2801)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…rgoproj#2806)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…ervicebus from 1.4.0 to 1.4.1 (argoproj#2808)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…roj#2807)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…goproj#2804)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
Signed-off-by: gokulav137 <gokulav999@gmail.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…oproj#2815)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…rgoproj#2816)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…68 (argoproj#2817)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
dependabot bot and others added 5 commits September 26, 2023 06:12
…roj#2818)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…rgoproj#2819)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
…proj#2821)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
Signed-off-by: Tommi Hovi <tommi.hovi@gmail.com>
Signed-off-by: jmillage <jmillage@bcubed-corp.com>
@joelcomp1 joelcomp1 marked this pull request as ready for review September 26, 2023 10:13
@@ -32,9 +32,6 @@ func ValidateEventBus(eb *v1alpha1.EventBus) error {
if x.Version == "" {
return fmt.Errorf("invalid spec: a version for jetstream needs to be specified")
}
if x.Replicas != nil && *x.Replicas < 3 {
Copy link
Member

Choose a reason for hiding this comment

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

To disallow replicas==2 or replicas<=0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes good point, I'll add that check now

@@ -436,7 +436,7 @@ func (i *natsInstaller) buildConfigMap() (*corev1.ConfigMap, error) {
ssName := generateStatefulSetName(i.eventBus)
replicas := i.eventBus.Spec.NATS.Native.GetReplicas()
if replicas < 3 {
replicas = 3
i.logger.Errorw("streaming recommends a replica size >= 3, '%v'", replicas)
Copy link
Member

Choose a reason for hiding this comment

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

Shall we keep the legacy nats streaming untouched, since it's already EOL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hadn't even thought about that, do you have a preference? Not sure how much NATS streaming is still used

Copy link
Member

Choose a reason for hiding this comment

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

This is the file for nats streaming installation, I would recommend to keep it untouched.

@@ -436,7 +436,7 @@ func (i *natsInstaller) buildConfigMap() (*corev1.ConfigMap, error) {
ssName := generateStatefulSetName(i.eventBus)
replicas := i.eventBus.Spec.NATS.Native.GetReplicas()
if replicas < 3 {
replicas = 3
i.logger.Errorw("streaming recommends a replica size >= 3, '%v'", replicas)
Copy link
Member

Choose a reason for hiding this comment

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

This is the file for nats streaming installation, I would recommend to keep it untouched.

replicas := i.eventBus.Spec.NATS.Native.Replicas
if replicas < 3 {
replicas = 3
i.logger.Errorw("streaming recommends a replica >= 3, '%v'", replicas)
Copy link
Member

Choose a reason for hiding this comment

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

Use Warnw, and make the log message a complete sentence.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reverted this back instead of fixing it since it was in the same file as above with the STAN implementation

@whynowy
Copy link
Member

whynowy commented Oct 3, 2023

Sorry for the late response!

@joelcomp1
Copy link
Contributor Author

reverted back the nats.go file to leave STAN untouched.

Copy link
Member

@whynowy whynowy left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@whynowy whynowy merged commit 4f01b34 into argoproj:master Oct 5, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow Jetstream have a replica of 1
7 participants