Skip to content
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

improve kic engine #6441

Merged
merged 16 commits into from
Jan 31, 2020
Merged

improve kic engine #6441

merged 16 commits into from
Jan 31, 2020

Conversation

medyagh
Copy link
Member

@medyagh medyagh commented Jan 30, 2020

This PR is in a mergable state, please feel free to merge in my absense if it looks good to you.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 30, 2020
@medyagh medyagh changed the title wip: detect all containers created by kic wip: improve kic engine Jan 30, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: medyagh

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 30, 2020
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 31, 2020
@codecov-io
Copy link

codecov-io commented Jan 31, 2020

Codecov Report

Merging #6441 into master will increase coverage by 0.01%.
The diff coverage is 52.38%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6441      +/-   ##
==========================================
+ Coverage   37.68%   37.69%   +0.01%     
==========================================
  Files         128      128              
  Lines        8654     8674      +20     
==========================================
+ Hits         3261     3270       +9     
- Misses       4970     4979       +9     
- Partials      423      425       +2
Impacted Files Coverage Δ
pkg/minikube/cluster/cluster.go 37.59% <0%> (-0.68%) ⬇️
pkg/minikube/config/profile.go 69.29% <84.61%> (+1.97%) ⬆️
cmd/minikube/cmd/delete.go 27.41% <0%> (-1.02%) ⬇️

Copy link
Collaborator

@sharifelgamal sharifelgamal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just some nits.

pkg/drivers/kic/types.go Outdated Show resolved Hide resolved
pkg/minikube/config/profile.go Outdated Show resolved Hide resolved
@medyagh
Copy link
Member Author

medyagh commented Jan 31, 2020

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jan 31, 2020
@minikube-pr-bot
Copy link

Error: running mkcmp: exit status 1

@minikube-pr-bot
Copy link

All Times minikube: [ 95.926872 94.470170 95.397701]
All Times Minikube (PR 6441): [ 95.428925 94.169041 95.835833]

Average Minikube (PR 6441): 95.144600
Average minikube: 95.264914

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 6441) |
+----------------------+-----------+--------------------+
| minikube v           |  0.298506 |           0.285694 |
| Creating kvm2        | 19.797973 |          20.663571 |
| Preparing Kubernetes | 49.787544 |          47.927057 |
| Pulling images       |  3.694858 |           4.041492 |
| Launching Kubernetes | 18.912916 |          19.449613 |
| Waiting for cluster  |  2.722737 |           2.725440 |
+----------------------+-----------+--------------------+

@minikube-pr-bot
Copy link

All Times minikube: [ 95.867790 93.286408 94.530464]
All Times Minikube (PR 6441): [ 96.199899 93.655959 92.400302]

Average minikube: 94.561554
Average Minikube (PR 6441): 94.085387

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 6441) |
+----------------------+-----------+--------------------+
| minikube v           |  0.308794 |           0.282818 |
| Creating kvm2        | 20.200925 |          20.248669 |
| Preparing Kubernetes | 49.225211 |          48.262123 |
| Pulling images       |  3.671031 |           3.961641 |
| Launching Kubernetes | 18.718038 |          18.369615 |
| Waiting for cluster  |  2.386084 |           2.910114 |
+----------------------+-----------+--------------------+

@medyagh
Copy link
Member Author

medyagh commented Jan 31, 2020

created an issue for the cache integeration test failure #6445

@medyagh medyagh changed the title wip: improve kic engine improve kic engine Jan 31, 2020
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 31, 2020
@minikube-pr-bot
Copy link

All Times minikube: [ 93.923070 94.966001 95.073241]
All Times Minikube (PR 6441): [ 96.417049 94.450692 93.664918]

Average Minikube (PR 6441): 94.844220
Average minikube: 94.654104

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 6441) |
+----------------------+-----------+--------------------+
| minikube v           |  0.300202 |           0.288106 |
| Creating kvm2        | 19.906288 |          19.708887 |
| Preparing Kubernetes | 48.989767 |          48.782428 |
| Pulling images       |  3.610835 |           3.940971 |
| Launching Kubernetes | 18.909269 |          19.344339 |
| Waiting for cluster  |  2.887913 |           2.729068 |
+----------------------+-----------+--------------------+

@minikube-pr-bot
Copy link

All Times minikube: [ 95.416723 93.655667 96.011758]
All Times Minikube (PR 6441): [ 94.323470 94.733609 93.840848]

Average minikube: 95.028049
Average Minikube (PR 6441): 94.299309

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 6441) |
+----------------------+-----------+--------------------+
| minikube v           |  0.292774 |           0.289619 |
| Creating kvm2        | 19.744815 |          19.628591 |
| Preparing Kubernetes | 49.001844 |          49.076052 |
| Pulling images       |  3.615937 |           3.505789 |
| Launching Kubernetes | 19.603702 |          19.346196 |
| Waiting for cluster  |  2.718299 |           2.401863 |
+----------------------+-----------+--------------------+

@medyagh medyagh requested a review from tstromberg January 31, 2020 05:38
@minikube-pr-bot
Copy link

All Times minikube: [ 96.728102 97.170769 96.069193]
All Times Minikube (PR 6441): [ 94.899581 93.708836 94.595336]

Average minikube: 96.656021
Average Minikube (PR 6441): 94.401251

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 6441) |
+----------------------+-----------+--------------------+
| minikube v           |  0.310356 |           0.285199 |
| Creating kvm2        | 20.892773 |          19.708333 |
| Preparing Kubernetes | 49.128437 |          48.763428 |
| Pulling images       |  3.800456 |           3.815341 |
| Launching Kubernetes | 20.094277 |          18.881442 |
| Waiting for cluster  |  2.381710 |           2.898709 |
+----------------------+-----------+--------------------+

@@ -165,6 +174,26 @@ func ListProfiles(miniHome ...string) (validPs []*Profile, inValidPs []*Profile,
return validPs, inValidPs, nil
}

// removeDupes removes duplicates
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is simple enough to just inline.


for n := range profiles {
if seen[profiles[n]] {
// Do not add duplicate.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of an else, use a continue

@tstromberg tstromberg merged commit d302b84 into kubernetes:master Jan 31, 2020
@medyagh medyagh deleted the kic_engine branch May 2, 2020 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
6 participants