-
Notifications
You must be signed in to change notification settings - Fork 40
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: event logs clean #256
Conversation
Signed-off-by: xu.jingwen <xu.jingwen@99cloud.net>
@Xvv-v: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Signed-off-by: xu.jingwen <xu.jingwen@99cloud.net>
Codecov Report
@@ Coverage Diff @@
## master #256 +/- ##
==========================================
- Coverage 13.12% 13.11% -0.01%
==========================================
Files 98 98
Lines 15341 15359 +18
==========================================
+ Hits 2014 2015 +1
- Misses 13088 13105 +17
Partials 239 239
|
/cc @x893675 |
/cc @lixd |
/milestone v1.3.1 |
pkg/models/models.go
Outdated
@@ -168,6 +169,14 @@ func ObjectMetaFilter(obj metav1.ObjectMeta, key, value string) bool { | |||
if !strings.Contains(obj.Annotations[common.AnnotationDescription], value) { | |||
return false | |||
} | |||
case "time": |
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.
rename create_before
pkg/models/platform/interface.go
Outdated
@@ -57,6 +57,7 @@ type EventReader interface { | |||
type EventReaderEx interface { | |||
GetEventEx(ctx context.Context, name string, resourceVersion string) (*v1.Event, error) | |||
ListEventsEx(ctx context.Context, query *query.Query) (*models.PageableResponse, error) | |||
ListEventsWithTimeEx(ctx context.Context, query *query.Query) (*models.PageableResponse, error) |
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.
remove
@@ -138,6 +138,30 @@ func (p *platformOperator) ListEventsEx(ctx context.Context, query *query.Query) | |||
return models.ListExV2(ctx, p.eventStorage, query, p.eventFilter, nil, nil) | |||
} | |||
|
|||
func (p *platformOperator) ListEventsWithTimeEx(ctx context.Context, query *query.Query) (*models.PageableResponse, error) { |
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.
remove
pkg/controller/audit_status.go
Outdated
q := &query.Query{ | ||
FieldSelector: fieldSelector, | ||
Reverse: false, | ||
Limit: -1, |
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.
remove Limit
pkg/controller/audit_status.go
Outdated
FieldSelector: fieldSelector, | ||
Reverse: false, | ||
Limit: -1, | ||
Pagination: &query.Pagination{ |
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.
use query.NoPagination()
pkg/controller/audit_status.go
Outdated
Offset: 0, | ||
}, | ||
} | ||
loginLogs, err := s.AuditOperator.ListEvents(context.TODO(), q) |
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.
query with timestamp, then check entry size, if not allowd, skip direct
pkg/controller/audit_status.go
Outdated
q.FuzzySearch = map[string]string{ | ||
"time": timestamp, | ||
} | ||
eventList, err := s.AuditOperator.ListEventsWithTimeEx(context.TODO(), q) |
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.
use ListEventEx instead
pkg/auditing/option/options.go
Outdated
) | ||
|
||
type AuditOptions struct { | ||
EventLogHistoryRetentionPeriod time.Duration `json:"eventLogHistoryRetentionPeriod" yaml:"eventLogHistoryRetentionPeriod"` |
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.
rename RetentionPeriod
rename MaximumEntries
pkg/controller/audit_status.go
Outdated
|
||
func (s *AuditStatusMon) monitorAuditStatus() { | ||
timestamp := time.Now().Add(-s.AuditOptions.EventLogHistoryRetentionPeriod).Format(time.RFC3339) | ||
s.cleanLogs("type", timestamp) |
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.
add annotate
// clean login / logout audit envent
s.cleanLogs("type", timestamp)
pkg/auditing/option/options.go
Outdated
default: | ||
return errors.New("[--period] format error, only support [h, d, w]") | ||
} | ||
} else { |
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.
return error first
if !m.MatchString(o.period){
return errors.New("only support [ None | Metadata | Request | RequestResponse ]")
}
// do parse
pkg/controller/audit_status.go
Outdated
|
||
func (s *AuditStatusMon) monitorAuditStatus() { | ||
timestamp := time.Now().Add(-s.AuditOptions.EventLogHistoryRetentionPeriod).Format(time.RFC3339) | ||
s.cleanLogs("type", timestamp) |
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.
wrong FieldSelector format
Signed-off-by: xu.jingwen <xu.jingwen@99cloud.net>
Signed-off-by: xu.jingwen <xu.jingwen@99cloud.net>
var errors []error | ||
errors = append(errors, s.GenericServerRunOptions.Validate()...) | ||
errors = append(errors, s.EtcdOptions.Validate()...) | ||
errors = append(errors, s.MQOptions.Validate()...) | ||
errors = append(errors, s.LogOptions.Validate()...) | ||
errors = append(errors, s.AuthenticationOptions.Validate()...) | ||
if err = s.AuditOptions.Validate(); err != nil { |
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.
rewrite to append(errors, s.AuditOptions.Validate())
pkg/server/config/config.go
Outdated
@@ -56,6 +58,7 @@ type Config struct { | |||
MQOptions *natsio.NatsOptions `json:"mq,omitempty" yaml:"mq,omitempty" mapstructure:"mq"` | |||
LogOptions *logger.Options `json:"log,omitempty" yaml:"log,omitempty" mapstructure:"log"` | |||
AuthenticationOptions *authoptions.AuthenticationOptions `json:"authentication,omitempty" yaml:"authentication,omitempty" mapstructure:"authentication"` | |||
AuditOptions *auditoptions.AuditOptions `json:"auditOptions,omitempty" yaml:"auditOptions,omitempty" mapstructure:"authentication"` |
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.
audit tag not same with kubeclipper-server.yaml
Signed-off-by: xu.jingwen <xu.jingwen@99cloud.net>
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.
/lgtm
LGTM label has been added. Git tree hash: 85035ebb996ea51c7bfd8faa5c1b94678c4a3645
|
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lixd, x893675, Xvv-v The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: xu.jingwen xu.jingwen@99cloud.net
What type of PR is this?
/kind feature
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #255
Special notes for reviewers:
Does this PR introduced a user-facing change?
Additional documentation, usage docs, etc.: