search

nats-io / nats-streaming-operator

NATS Streaming Operator
Apache License 2.0
174 stars 44 forks source link

workaround issue #61 - adding missing configs to all examples of cluster mode #76

Closed hbobenicio closed 3 years ago

hbobenicio commented 3 years ago

According to #61 , if you have a spec.size > 1 and you're using a file store, if you do not define a spec.config, then the operator controller will fail to define the correct cluster_node_id for the CLI command of the container. IMHO the best fix is to change that logic in order to allow empty configs to just work by setting the correct cluster_node_id anyways. But for now, docs should at least not mislead users to this "misconfiguration".

hbobenicio commented 3 years ago

@wallyqs If you prefer to not change the spec model to require a config key and instead fix the logic, I'm happy closing this PR to open a new one with the fix. The relevant piece of code is here (from controller.go):

func stanContainerCmd(o *stanv1alpha1.NatsStreamingCluster, pod *k8scorev1.Pod) []string {
    // ...
    if o.Spec.StoreType == "SQL" {
        // ...
    }  else if o.Spec.StoreType == "MEMORY" {
        // ...
    } else {
        // ...

        // FIX ME.
        // If o.Spec.Config is nil, isClustered := false.
        // This will prevent the condition below to set --cluster_node_id properly, even though o.Spec.Size > 1.
        // May it safely assume cluster mode when o.Spec.Size > 1?
        isClustered := o.Spec.Config != nil && (o.Spec.Size > 1 || o.Spec.Config.Clustered)

    if isClustered && !ftModeEnabled {
        storeArgs = append(storeArgs, "-clustered")
        storeArgs = append(storeArgs, fmt.Sprintf("--cluster_node_id=%q", pod.Name))
    }
    }
}
wallyqs commented 3 years ago

@hbobenicio OK I will merge the workaround as it already helps and proper fix can be done in separate. Thanks a lot for finding the bug!