fix: ingress class not effect in resource sync logic #1311
Conversation
pkg/providers/ingress/ingress.go
Outdated
| if valid { | ||
| log.Debugw("ingress add event arrived", | ||
| zap.Any("object", obj), | ||
| ) | ||
| } else { | ||
| log.Debugw("ignore noneffective ingress add event", | ||
| zap.Any("object", obj), | ||
| ) | ||
| return | ||
| } |
There was a problem hiding this comment.
We can simplify the code here, right?
There was a problem hiding this comment.
Are you means to summarize these "valid judge" logic to common codes?
There was a problem hiding this comment.
yes, we don't need judge valid=true and print log.
There was a problem hiding this comment.
yes, we don't need judge
valid=trueand print log.
Ok, i'll modify it
tao12345666333
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1311 +/- ##
==========================================
- Coverage 39.80% 39.76% -0.04%
==========================================
Files 75 75
Lines 6962 6969 +7
==========================================
Hits 2771 2771
- Misses 3892 3899 +7
Partials 299 299
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
pkg/providers/ingress/ingress.go
Outdated
| zap.Any("object", obj), | ||
| ) | ||
| } else { | ||
| if !c.isIngressEffective(ing) { |
There was a problem hiding this comment.
These places do not need to be changed, and the debug log needs to be kept.
pkg/providers/ingress/ingress.go
Outdated
| zap.Any("old object", oldObj), | ||
| ) | ||
| } else { | ||
| if !c.isIngressEffective(curr) { |
pkg/providers/ingress/ingress.go
Outdated
| ing := kube.MustNewIngress(obj) | ||
| if !c.isIngressEffective(ing) { | ||
| log.Debugw("ignore noneffective ingress add event", |
There was a problem hiding this comment.
Only the log of the sync object should be printed, and the sync should not care about the ingresssclass log.
There was a problem hiding this comment.
I misunderstanding something. And i will modify it soon
|
I agree with @AlinsRan , in this PR you can just keep the changes in the ResourceSync function, which will help me cherry-pick it to other versions. |
Co-authored-by: Xin Rong <1324266492@qq.com>
Co-authored-by: Sarasa Kisaragi <lingsamuelgrace@gmail.com> Co-authored-by: Xin Rong <1324266492@qq.com>
Type of change:
What this PR does / why we need it:
fix #1310
Pre-submission checklist: