Fix wrong skipped message of linkerd inject for a namespace#11179
Merged
alpeb merged 3 commits intolinkerd:mainfrom Sep 6, 2023
Merged
Fix wrong skipped message of linkerd inject for a namespace#11179alpeb merged 3 commits intolinkerd:mainfrom
skipped message of linkerd inject for a namespace#11179alpeb merged 3 commits intolinkerd:mainfrom
Conversation
Signed-off-by: Takumi Sue <[email protected]>
alpeb
reviewed
Jul 31, 2023
|
|
||
| ‼ "linkerd.io/inject: disabled" annotation set on deployment/web | ||
|
|
||
| deployment "web" skipped |
Member
There was a problem hiding this comment.
This should still being reported as skipped... I think my recommendation in #10257 was off. Can you explore the UnsupportedResource field in report.go for it to properly consider namespaces? Things to keep in mind for example would be that linkerd inject --manual should still only add an annotation in this particular case (as opposed to adding the proxy yaml like it does for workloads).
Signed-off-by: Takumi Sue <[email protected]>
alpeb
reviewed
Aug 8, 2023
Member
alpeb
left a comment
alpeb
left a comment
There was a problem hiding this comment.
Great fix! Just one tiny nit below.
cli/cmd/inject.go
Outdated
Comment on lines
183
to
184
| if conf.HasPodTemplate() { | ||
| conf.AppendPodAnnotations(rt.overrideAnnotations) | ||
| report.Annotated = true | ||
| if len(rt.overrideAnnotations) > 0 { |
Member
There was a problem hiding this comment.
This can be collapsed into a single if statement
Signed-off-by: Takumi Sue <[email protected]>
alpeb
approved these changes
Aug 9, 2023
mikutas
changed the title
report's field updateskipped message of linkerd inject for a namespace
mateiidavid
approved these changes
Sep 6, 2023
adamshawvipps
pushed a commit
to adamshawvipps/linkerd2
that referenced
this pull request
Sep 18, 2023
…erd#11179) Signed-off-by: Takumi Sue <[email protected]>
adamshawvipps
pushed a commit
to adamshawvipps/linkerd2
that referenced
this pull request
Sep 18, 2023
…erd#11179) Signed-off-by: Takumi Sue <[email protected]> Signed-off-by: Adam Shaw <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #10231