-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
fix: custom crd not printing streams logs #9136
fix: custom crd not printing streams logs #9136
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #9136 +/- ##
==========================================
- Coverage 70.48% 63.71% -6.77%
==========================================
Files 515 630 +115
Lines 23150 32447 +9297
==========================================
+ Hits 16317 20674 +4357
- Misses 5776 10183 +4407
- Partials 1057 1590 +533 ☔ View full report in Codecov by Sentry. |
fca1c2b
to
b4a1a0b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @beastpu! Thanks for opening this PR. I left a comment for one more place we need to update the resource lists.
Checking the change, I think I found a case where still we'll not be able to get the logs: when the CRD uses a field different than image
to define the image to deploy. When we are going through the manifests to get the images, Skaffold is looking for the specific image
field:
skaffold/pkg/skaffold/kubernetes/manifest/images.go
Lines 114 to 117 in 1f591ba
func (is *imageSaver) Visit(gk apimachinery.GroupKind, navpath string, o map[string]interface{}, k string, v interface{}, rs ResourceSelector) bool { | |
if k != imageField { | |
return true | |
} |
I think the proper way here would be to use the selectors defined in the resourceSelector
from the skaffold.yaml
.
With that said, I'll create another issue to track that case and tackle that in another PR; I think we can merge this PR once we have the last change from the comment I left. Thanks a lot for your help! 😄
Description
my company uses a custom CRD (openkruise https://openkruise.io/) to manage workloads.
During local development, we discovered that aggregated logs were not being printed. Aggregated logs are crucial for microservices architecture.
skaffold.yaml
sts.yaml
User facing changes (remove if N/A)
I found that the reason was the absence of corresponding GVK (GroupVersionKind) in
manifest.TransformAllowlist
andmanifest.TransformDenylist
.My solution is to pass ConsolidateTransformConfiguration into the NewResourceSelectorImages method because we can configure resourceSelector in the skaffold.yaml file.