Futher storage isolation and removal of the tools interface. #18383
Conversation
k8s-github-robot
added
the
size/L
|
Labelling this PR as size/L |
timothysc
force-pushed
the
tools_removal
branch
from
ebb768d to
57ab3e3
Compare
|
Will take a look tomorrow. |
|
Why do we have a forked go-etcd? |
|
@karlkfi - The fork has nothing to do with changes to your code, they are shuffling to isolate the dependency which existed in the startup of the apiserver. Post shuffle, the dependencies will be upgraded &| eliminated, this is more about encapsulation to enable other work. The modifications to the mesos code are simply to remove the tools dependency. |
|
GCE e2e test build/test passed for commit ebb768df52c40554ccf2fda69e9cb1fe0b6cc367. |
|
GCE e2e test build/test passed for commit 57ab3e354af274a0c957e381b87a40ff4650bd1a. |
| if !reflect.DeepEqual(client.calls, expectedCalls) { | ||
| t.Errorf("unexpected calls: %#v", client.calls) | ||
| } | ||
| w_ldr.Stop() |
There was a problem hiding this comment.
do we not need to show that the calls happened in the expected order?
There was a problem hiding this comment.
I would argue no, b/c the goal of the test is to check that (X) is not the leader.
Operational ordering imho was a false test that wrapped the tool.interface.
|
The changes outside mesos code lgtm (just very minor comments). |
timothysc
force-pushed
the
tools_removal
branch
from
57ab3e3 to
836dfd7
Compare
|
LGTM from mi side (I'm waiting for @karlkfi for confirmation if he is also fine with it) |
|
GCE e2e test build/test passed for commit 836dfd7ddd703dd37e2e13e8ee657ce31c442c02. |
|
lgtm |
|
@wojtek-t PTAL for lgtm apply. |
|
Generated docs are out of date. Please run hack/update-generated-docs.sh @timothysc - can you please fix it? |
timothysc
force-pushed
the
tools_removal
branch
from
836dfd7 to
413d8d1
Compare
|
@wojtek-t done. |
wojtek-t
added
the
lgtm
|
LGTM |
|
GCE e2e test build/test passed for commit 413d8d1. |
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e test build/test passed for commit 413d8d1. |
|
Automatic merge from submit-queue |
Auto commit by PR queue bot
| @@ -234,7 +230,6 @@ func (s *APIServer) AddFlags(fs *pflag.FlagSet) { | |||
| fs.StringVar(&s.AdmissionControl, "admission-control", s.AdmissionControl, "Ordered list of plug-ins to do admission control of resources into cluster. Comma-delimited list of: "+strings.Join(admission.GetPlugins(), ", ")) | |||
| fs.StringVar(&s.AdmissionControlConfigFile, "admission-control-config-file", s.AdmissionControlConfigFile, "File with admission control configuration.") | |||
| fs.StringSliceVar(&s.EtcdServerList, "etcd-servers", s.EtcdServerList, "List of etcd servers to watch (http://ip:port), comma separated. Mutually exclusive with -etcd-config") | |||
| fs.StringVar(&s.EtcdConfigFile, "etcd-config", s.EtcdConfigFile, "The config file for the etcd client. Mutually exclusive with -etcd-servers.") | |||
There was a problem hiding this comment.
This is a breaking change... we now have no way to run against a secured etcd. I know the client lib no longer supports the config file, but we have to (even if it means deserializing the file and building the etcd client config ourselves)
This completes the removal of the tools.interface and adds mechanics to allow for multiple back-ends via a Config interface.
@wojtek-t & @karlkfi | @jdef for review, as it touches their areas.