Skip to content

Commit

Permalink
ListParams now default enables bookmarks - closes #226
Browse files Browse the repository at this point in the history
Since this flag does nothing for older apiservers it's safe to default
enable it. This will also close #219 in the process.

Note that ListParams does not pick up on the flag for non-watch calls.
  • Loading branch information
clux committed Feb 28, 2021
1 parent e4196d5 commit 72aad07
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 8 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
UNRELEASED
===================
* see https://github.com/clux/kube-rs/compare/0.51.0...master
* `kube`: BREAKING: ListParams bookmarks default enabled- #226 via
- renames `ListParams::allow_bookmarks` to `ListParams::bookmarks`
- `ListParams::default()` sets `bookmark` to true to avoid bad bad defaults
- `ListParams::allow_bookmarks` replaced by `ListParams::disable_bookmarks`

0.51.0 / 2021-02-28
===================
Expand Down
2 changes: 1 addition & 1 deletion examples/configmap_watcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ async fn main() -> anyhow::Result<()> {
let namespace = std::env::var("NAMESPACE").unwrap_or("default".into());

let cms: Api<ConfigMap> = Api::namespaced(client, &namespace);
let lp = ListParams::default().allow_bookmarks();
let lp = ListParams::default();

let mut w = watcher(cms, lp).boxed();
while let Some(event) = w.try_next().await? {
Expand Down
28 changes: 23 additions & 5 deletions kube/src/api/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::{Error, Result};
use serde::Serialize;

/// Common query parameters used in watch/list/delete calls on collections
#[derive(Default, Clone)]
#[derive(Clone)]
#[allow(missing_docs)]
pub struct ListParams {
/// A selector to restrict the list of returned objects by their labels.
Expand Down Expand Up @@ -32,7 +32,7 @@ pub struct ListParams {
/// If this is not a watch, this field is ignored.
/// If the feature gate WatchBookmarks is not enabled in apiserver,
/// this field is ignored.
pub allow_bookmarks: bool,
pub bookmarks: bool,

/// Limit the number of results.
///
Expand All @@ -47,6 +47,21 @@ pub struct ListParams {
pub continue_token: Option<String>,
}

impl Default for ListParams {
fn default() -> Self {
Self {
// bookmarks stable since 1.17, and backwards compatible
bookmarks: true,

label_selector: None,
field_selector: None,
timeout: None,
limit: None,
continue_token: None,
}
}
}

impl ListParams {
pub(crate) fn validate(&self) -> Result<()> {
if let Some(to) = &self.timeout {
Expand Down Expand Up @@ -99,9 +114,12 @@ impl ListParams {
self
}

/// Enables watch bookmarks from the api server if supported
pub fn allow_bookmarks(mut self) -> Self {
self.allow_bookmarks = true;
/// Disables watch bookmarks to simplify watch handling
///
/// This is not recommended to use with production watchers as it can cause desyncs.
/// See [#219](https://github.com/clux/kube-rs/issues/219) for details.
pub fn disable_bookmarks(mut self) -> Self {
self.bookmarks = false;
self
}

Expand Down
4 changes: 2 additions & 2 deletions kube/src/api/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl Resource {
if let Some(labels) = &lp.label_selector {
qp.append_pair("labelSelector", &labels);
}
if lp.allow_bookmarks {
if lp.bookmarks {
qp.append_pair("allowWatchBookmarks", "true");
}

Expand Down Expand Up @@ -474,7 +474,7 @@ mod test {
let req = r.watch(&gp, "0").unwrap();
assert_eq!(
req.uri(),
"/api/v1/namespaces/ns/pods?&watch=true&resourceVersion=0&timeoutSeconds=290"
"/api/v1/namespaces/ns/pods?&watch=true&resourceVersion=0&timeoutSeconds=290&allowWatchBookmarks=true"
);
}
#[test]
Expand Down

0 comments on commit 72aad07

Please sign in to comment.