-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Write commands to audit log regardless of whether it terminates cleanly #13307
Conversation
Can one of the admins verify this patch? |
You can check for these failures locally by running |
@spowelljr Yes thanks, I am working on them. |
25466d3
to
b44b4b9
Compare
pkg/minikube/audit/audit.go
Outdated
return fmt.Errorf("failed to set the log file: %v", err) | ||
} | ||
} | ||
auditLogs, err := logsToRows(currentLogFile) |
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.
@spowelljr How can we convert currentLogFile
from *os.File
type to []string
type ?
We can use ioutil.ReadFile()
function for converting it to []byte
and then string
but how can we convert it to []string
type so that we can convert to []row
through logsToRows()
function.
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.
You can use https://pkg.go.dev/encoding/json#Unmarshal to convert []byte
to a struct (row
).
e8c33e4
to
16343ff
Compare
@spowelljr We are getting error where funcs
Could you please give your thoughts on this. |
Those are the audit test files that are giving you errors, they need to updated as the func changes. |
c7159e7
to
006fb21
Compare
@spowelljr |
ce4c6f3
to
daeb06f
Compare
daeb06f
to
40b2e50
Compare
timing minikube failed, please try again |
These are the flake rates of all failed tests.
Too many tests failed - See test logs for more details. To see the flake rates of all tests by environment, click here. |
kvm2 driver with docker runtime
Times for minikube start: 53.9s 50.1s 51.4s 51.7s 51.4s Times for minikube ingress: 29.6s 26.6s 29.7s 26.1s 28.6s docker driver with docker runtime |
kvm2 driver with docker runtime
Times for minikube (PR 13307) ingress: 29.3s 29.3s 28.4s 25.3s 28.3s Times for minikube start: 52.8s 50.9s 51.1s 52.2s 52.4s docker driver with docker runtime
Times for minikube start: 52.6s 24.0s 25.2s 24.6s 25.1s Times for minikube ingress: 22.9s 22.4s 21.9s 22.4s 83.4s docker driver with containerd runtime
Times for minikube start: 29.8s 32.4s 34.2s 40.2s 29.7s Times for minikube ingress: 22.4s 17.9s 18.4s 22.4s 22.0s |
kvm2 driver with docker runtime
Times for minikube ingress: 25.6s 27.1s 29.1s 59.6s 30.6s Times for minikube start: 53.8s 50.7s 50.6s 51.5s 51.7s docker driver with docker runtime |
These are the flake rates of all failed tests.
Too many tests failed - See test logs for more details. To see the flake rates of all tests by environment, click here. |
These are the flake rates of all failed tests.
Too many tests failed - See test logs for more details. To see the flake rates of all tests by environment, click here. |
These are the flake rates of all failed tests.
Too many tests failed - See test logs for more details. To see the flake rates of all tests by environment, click here. |
The problem is we were leaving the audit log file for the entirety of the command run. The issue is Windows can only truncate a file when it's closed, so if you have more than one command running at the same time (always happening during testing) if one command completes it tried to truncate the log but the other command has it open so the truncation fails. Refactored the code so we only have the audit log open for the minimum required amount of time required before closing it again. |
kvm2 driver with docker runtime
Times for minikube start: 50.7s 52.7s 51.5s 50.3s 51.5s Times for minikube (PR 13307) ingress: 25.1s 25.1s 30.1s 25.2s 29.6s docker driver with docker runtime
Times for minikube start: 24.1s 23.9s 24.3s 23.6s 23.7s Times for minikube (PR 13307) ingress: 22.1s 22.0s 20.0s 23.0s 22.0s docker driver with containerd runtime
Times for minikube (PR 13307) ingress: 28.0s 16.9s 22.5s 31.5s 18.0s Times for minikube start: 32.4s 28.2s 44.0s 28.9s 43.9s |
These are the flake rates of all failed tests.
Too many tests failed - See test logs for more details. To see the flake rates of all tests by environment, click here. |
… it is finished or not
kvm2 driver with docker runtime
Times for minikube ingress: 25.6s 26.1s 26.5s 29.0s 28.0s Times for minikube start: 51.7s 52.2s 50.9s 52.3s 50.6s docker driver with docker runtime
Times for minikube ingress: 25.4s 23.4s 23.9s 24.9s 23.4s Times for minikube start: 24.6s 24.9s 24.6s 24.2s 23.9s docker driver with containerd runtime
Times for minikube ingress: 18.4s 22.4s 32.4s 22.9s 22.9s Times for minikube start: 43.7s 28.3s 28.3s 32.9s 29.1s |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: NikhilSharmaWe, sharifelgamal 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 |
Fixes #13186
Before (note start entry is not present):
After (note start entry is present):