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

Update example config #894

Merged
merged 1 commit into from
Nov 11, 2024
Merged

Conversation

brianrudolf
Copy link
Contributor

@brianrudolf brianrudolf commented Nov 6, 2024

Correct a nodePool component to use a dash - instead of an underscore _ as this string is used in a web address and needs to be URL compliant.

Description

This makes a minor change to the component name in an example manifest. Using an underscore causes the operator to fail to create the resource with the following error message:

{
    "level": "error",
    "msg": "Reconciler error",
    "controller": "opensearchcluster",
    "controllerGroup": "opensearch.opster.io",
    "controllerKind": "OpenSearchCluster",
    "OpenSearchCluster": {
        "name": "central-logging-cluster-manager",
        "namespace": "monitoring"
    },
    "namespace": "monitoring",
    "name": "central-logging-cluster-manager",
    "error": "failed to create resource: creating resource failed: Service \"central-logging-opensearch-cluster_manager\" is invalid: metadata.name: Invalid value: \"central-logging-opensearch-cluster_manager\": a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name',  or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?');  failed to create resource: creating resource failed: StatefulSet.apps \"central-logging-cluster-manager-cluster_manager\" is invalid: metadata.name: Invalid value: \"central-logging-cluster-manager-cluster_manager\": a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name',  or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')"
}

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].

Check List

  • Commits are signed per the DCO using --signoff
  • Unittest added for the new/changed functionality and all unit tests are successful
  • Customer-visible features documented
  • No linter warnings (make lint)

If CRDs are changed:

  • CRD YAMLs updated (make manifests) and also copied into the helm chart
  • Changes to CRDs documented

Please refer to the PR guidelines before submitting this pull request.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Correct a `nodePool` component to use a dash `-` instead of an underscore `_` as this string is used in a web address and needs to be URL compliant.

Signed-off-by: Brian White <44818189+brianrudolf@users.noreply.github.com>
@swoehrl-mw
Copy link
Collaborator

Hi @brianrudolf. Could you please elaborate and give a bit of context where the nodePool component is used in a web address?

@brianrudolf
Copy link
Contributor Author

@swoehrl-mw apologies I didn't think a draft PR would get attention until I had a chance to add the error message.
The error message I've added to the PR description is from the opensearch-k8s-operator indicating that the nodePool component is used as the name of the underlying Kubernetes Statefulset and must adhere to "a DNS-1035 label".

I made the code change in the web UI, which I now realize isn't signed. Not sure if this is an issue for a docs change.

Copy link
Collaborator

@swoehrl-mw swoehrl-mw left a comment

Choose a reason for hiding this comment

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

@brianrudolf Thanks for noticing this and filing the PR for it.

@swoehrl-mw swoehrl-mw merged commit e909161 into opensearch-project:main Nov 11, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants