Automatic merge from submit-queue
Move HighWaterMark to the top of the struct in order to fix arm, second time
ref: #33117
Sorry for not fixing everyone at once, but I seriously wasn't prepared for that quick LGTM 😄, so here's the other half.
@lavalamp
> lgtm, but seriously, this is terrible, we probably have this bug all over. And what if someone embeds the etcdWatcher struct in something else not at the top? We need the compiler to enforce things like this, it just can't be done manually. Can you file or link a golang issue for this?
I totally agree! There isn't currently a way of programmatically detecting this unfortunately.
I guess @davecheney or @minux can explain better to you why it's so hard.
This is noted in https://github.com/kubernetes/kubernetes/blob/master/docs/proposals/multi-platform.md as a corner case indeed.
@pwittrock This should be cherrypicked toghether with #33117
Automatic merge from submit-queue
stop etcd watcher when watch chan is closed
When startWatching() stops due to the watch chan is closed, the watcher could still keeps running in the background. When this case happen, we should stop the watcher entirely and close ResultChan() too.
Automatic merge from submit-queue
Split path validation into a separate library
This PR splits path segment validation into it's own package. This cuts off one of the restclient's dependency paths to some docker packages, and completely eliminates its dependency on go-restful swagger validation.
cc @kubernetes/sig-api-machinery
Automatic merge from submit-queue
pkg/storage: remove Codec() from interface
What?
Removes Codec() from storage.Interface.
Why?
- storage interface doesn't need to expose Codec().
- Codec() isn't used anywhere.
Automatic merge from submit-queue
cacher.go: remove NewCacher func
NewCacher is a wrapper of NewCacherFromConfig. NewCacher understands
how to create a key func from scopeStrategy. However, it is not the
responsibility of cacher. So we should remove this function, and
construct the config in its caller, which should understand scopeStrategy.
Automatic merge from submit-queue
pkg/storage/etcd3: remove name field in test
Current test gets the name with its test table index,
so there seems to be no reason to have name field in test table.
NewCacher is a wrapper of NewCacherFromConfig. NewCacher understands
how to create a key func from scopeStrategy. However, it is not the
responsibility of cacher. So we should remove this function, and
construct the config in its caller, which should understand scopeStrategy.
Automatic merge from submit-queue
kube-apiserver options should be decoupled from impls
A few months ago we refactored options to keep it independent of the
implementations, so that it could be used in CLI tools to validate
config or to generate config, without pulling in the full dependency
tree of the master. This change restores that by separating
server_run_options.go back to its own package.
Also, options structs should never contain non-serializable types, which
storagebackend.Config was doing with runtime.Codec. Split the codec out.
Fix a typo on the name of the etcd2.go storage backend.
Finally, move DefaultStorageMediaType to server_run_options.
@nikhiljindal as per my comment in #24454, @liggitt because you and I
discussed this last time
A few months ago we refactored options to keep it independent of the
implementations, so that it could be used in CLI tools to validate
config or to generate config, without pulling in the full dependency
tree of the master. This change restores that by separating
server_run_options.go back to its own package.
Also, options structs should never contain non-serializable types, which
storagebackend.Config was doing with runtime.Codec. Split the codec out.
Fix a typo on the name of the etcd2.go storage backend.
Finally, move DefaultStorageMediaType to server_run_options.
Automatic merge from submit-queue
etcd_watcher: make Deleted Event.Object's version consistent
### What's the problem?
In [sendDelete()](995f022808/pkg/storage/etcd/etcd_watcher.go (L437-L442)), Deleted Event.Object's resource version will be set to the latest resource version. This is actually an assumption made by cacher that all later events should have objects with larger verions; See [here](995f022808/pkg/storage/cacher.go (L579-L581)).
In sendModify(), it could also return Deleted event. However, the resource version is still the old version, which is inconsistent behavior with sendDelete().
### What's this PR?
This PR sets the version of oldObj in sendModify() to be the latest version.
Provided unit test for it.
Automatic merge from submit-queue
Return 'too old' errors from watch cache via watch stream
Fixes#25151
This PR updates the API server to produce the same results when a watch is attempted with a resourceVersion that is too old, regardless of whether the etcd watch cache is enabled. The expected result is a `200` http status, with a single watch event of type `ERROR`. Previously, the watch cache would deliver a `410` http response.
This is the uncached watch impl:
```
// Implements storage.Interface.
func (h *etcdHelper) WatchList(ctx context.Context, key string, resourceVersion string, filter storage.FilterFunc) (watch.Interface, error) {
if ctx == nil {
glog.Errorf("Context is nil")
}
watchRV, err := storage.ParseWatchResourceVersion(resourceVersion)
if err != nil {
return nil, err
}
key = h.prefixEtcdKey(key)
w := newEtcdWatcher(true, h.quorum, exceptKey(key), filter, h.codec, h.versioner, nil, h)
go w.etcdWatch(ctx, h.etcdKeysAPI, key, watchRV)
return w, nil
}
```
once the resourceVersion parses, there is no path that returns a direct error, so all errors would have to be returned as an `ERROR` event via the ResultChan().
