-
Notifications
You must be signed in to change notification settings - Fork 351
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
feat: add event & status for ApisixRoute v2 #386
Conversation
Codecov Report
@@ Coverage Diff @@
## master #386 +/- ##
==========================================
- Coverage 40.31% 39.05% -1.27%
==========================================
Files 39 39
Lines 3264 3326 +62
==========================================
- Hits 1316 1299 -17
- Misses 1785 1862 +77
- Partials 163 165 +2
Continue to review full report at Codecov.
|
pkg/ingress/apisix_route.go
Outdated
@@ -198,14 +212,50 @@ func (c *apisixRouteController) sync(ctx context.Context, ev *types.Event) error | |||
} | |||
|
|||
func (c *apisixRouteController) handleSyncErr(obj interface{}, err error) { | |||
event := obj.(*types.Event) | |||
route := event.Object.(kube.ApisixRouteEvent).OldObject |
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.
Not all events have OldObject
field, it's set only in UPDATE event.
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.
Yes, will modify this.
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.
done
pkg/ingress/secret.go
Outdated
zap.Any("object", obj), | ||
) | ||
//log.Debugw("secret add event arrived", | ||
// zap.Any("object", obj), |
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.
It seems that zap.Any
outputs the cert
and pkey
in []bytes
and it's not human-friendly. I think we can just print the name
and namespace
of this secret, and do not print the body.
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.
Yes, can not print.
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.
if log "object" with zap.Any
, got error bad access: nil dereference
.
I changed logging field.
pkg/ingress/apisix_route.go
Outdated
"github.com/apache/apisix-ingress-controller/pkg/log" | ||
"github.com/apache/apisix-ingress-controller/pkg/types" | ||
apisixv1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1" | ||
) | ||
|
||
const _routeController = "RouteController" |
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.
I am wondering whether we need a finer granularity component in apisix-ingress-controller. The pod events reporters are "kubelet", "scheduler", it's a whole program, not a module inside it.
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.
Do you mean using ApisixIngress
is better?
@gxthrj The e2e CI failed. |
It is strange, scale endpoint getting error. |
Please answer these questions before submitting a pull request