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

add support for deploymentLabels #1707

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

Conversation

quintonm
Copy link

This PR adds support for configuring additional labels for the deployment using deploymentLabels in values.yaml.

Copy link
Member

@aryan9600 aryan9600 left a comment

Choose a reason for hiding this comment

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

thanks for this PR! could you also update the chart's README accordingly. i see that we missed out on the docs for podLabels, so we can do both in this PR as well

@KrylixZA
Copy link

KrylixZA commented Oct 8, 2024

Could this be used to address the issues raised in #1312?

@aryan9600
Copy link
Member

@KrylixZA i'm investigating that. i'd say if that needs to be addressed, we shall do so in a different PR, since they're completely different things.

@quintonm
Copy link
Author

@aryan9600 README has been updated. Should I make a similar update to docs/gitbook/install/flagger-install-on-kubernetes.md?

@aryan9600
Copy link
Member

Sure, that'd be great. Could you also specify the podLabels and deploymentLabels values in the table in charts/flagger/README.md?

@quintonm
Copy link
Author

Sure, that'd be great. Could you also specify the podLabels and deploymentLabels values in the table in charts/flagger/README.md?

done

@quintonm
Copy link
Author

quintonm commented Dec 3, 2024

@aryan9600 Any other changes needed?

@quintonm
Copy link
Author

@aryan9600 @stefanprodan - Just following up on this PR. Are any other changes needed? Thanks for taking the time to review!

Copy link
Member

@aryan9600 aryan9600 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 @quintonm and apologies for the delay

@quintonm
Copy link
Author

@aryan9600 - How do I trigger a build? It looks like one is needed before the PR can be merged.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 39.30%. Comparing base (febc327) to head (8d9b1ad).
Report is 18 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1707      +/-   ##
==========================================
+ Coverage   39.27%   39.30%   +0.03%     
==========================================
  Files         284      284              
  Lines       22379    22391      +12     
==========================================
+ Hits         8789     8801      +12     
  Misses      12643    12643              
  Partials      947      947              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@aryan9600 aryan9600 left a comment

Choose a reason for hiding this comment

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

@quintonm could you squash all the commits into one and then force push?

charts/flagger/values.yaml Show resolved Hide resolved
@quintonm quintonm force-pushed the main branch 2 times, most recently from e8cf645 to 87db1a3 Compare January 13, 2025 13:54
@quintonm
Copy link
Author

@aryan9600 done

@quintonm
Copy link
Author

@aryan9600 Is there anything else that I need to do before build-flagger action can be executed?

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.

5 participants