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

Simplify pod label config #23

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

sd109
Copy link

@sd109 sd109 commented Jun 10, 2024

User description

It seems that a similar change was accepted recently #14 but was then reverted in #16 (reason unclear). This has lead to the issue described in #20. I think both the stateful set selector labels and the service selector labels should just always take the same value as the pod labels to avoid future issues. Happy to discuss in more detail if required.


PR Type

Bug fix, Enhancement


Description

  • Updated the service and stateful set selectors to use podLabels instead of selectorLabels to ensure consistency and avoid future issues.
  • Removed the selectorLabels configuration from values.yaml.
  • Cleaned up trailing whitespace and improved formatting in YAML files.

Changes walkthrough 📝

Relevant files
Enhancement
service.yaml
Update service selector to use pod labels                               

charts/vespa/templates/service.yaml

  • Updated the service selector to use pod labels instead of selector
    labels.
  • Removed trailing whitespace.
  • +2/-2     
    statefulset.yaml
    Update stateful set selector to use pod labels                     

    charts/vespa/templates/statefulset.yaml

  • Updated the stateful set selector to use pod labels instead of
    selector labels.
  • Removed trailing whitespace.
  • +4/-4     
    Bug fix
    values.yaml
    Remove selectorLabels and use podLabels in configurations

    charts/vespa/values.yaml

  • Removed selectorLabels and updated configurations to use podLabels.
  • Cleaned up trailing whitespace and formatting.
  • +6/-10   

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5]

    2

    🧪 Relevant tests

    No

    🔒 Security concerns

    No

    ⚡ Key issues to review

    Consistency Check:
    Ensure that the change from selectorLabels to podLabels does not affect other components or configurations that might be expecting selectorLabels.

    Configuration Removal:
    The removal of selectorLabels from values.yaml should be thoroughly checked to ensure it does not break existing deployments that might rely on this configuration.

    Copy link

    PR Code Suggestions ✨

    No code suggestions found for PR.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant