fix return types of delete (can return a status object) - fixes #32

CHANGELOG.md

@@ -2,7 +2,8 @@
 ==================
   * Typed `Api` variant called `OpenApi` introduced (see crd_openapi example)
   * Revert `client.request` return type change (back to response only from pre-0.7.0 #28)
-  * TODO: FIX BUG #32
+  * `delete` now returns `Either<Object<P, U>, ApiStatus> - for bug#32
+  * `delete_collection` now returns `Either<ObjectList<Object<P, U>>, ApiStatus> - for bug#32
 
 0.7.0 / 2019-05-27
 ==================

Cargo.toml

@@ -1,15 +1,15 @@
 [package]
 name = "kube"
 version = "0.7.0"
-description = "Opinionated Kubernetes client with reflectors"
+description = "Kubernetes client in the style of client-go"
 authors = [
   "clux <sszynrae@gmail.com>",
   "ynqa <un.pensiero.vano@gmail.com>",
 ]
 license-file = "LICENSE"
 repository = "https://github.com/clux/kube-rs"
 readme = "README.md"
-keywords = ["kubernetes"]
+keywords = ["kubernetes", "client-go", "client-rust", "openapi", "reflector", "informer"]
 categories = ["web-programming::http-client"]
 edition = "2018"
 
@@ -27,6 +27,7 @@ http = "0.1.17"
 url = "1.7.2"
 log = "0.4.6"
 k8s-openapi = { version = "0.4.0", features = ["v1_13"], optional = true }
+either = "1.5.2"
 
 [features]
 default = []

Makefile

@@ -17,7 +17,7 @@ push-docs:
 .PHONY: doc build
 
 test:
-	cargo test
+	cargo test --all-features
 
 readme:
 	rustdoc README.md --test --edition=2018

README.md

@@ -4,9 +4,9 @@
 [![Client Support Level](https://img.shields.io/badge/kubernetes%20client-alpha-green.svg?style=plastic&colorA=306CE8)](http://bit.ly/kubernetes-client-support-badge)
 [![Crates.io](https://img.shields.io/crates/v/kube.svg)](https://crates.io/crates/kube)
 
-Rust client for [Kubernetes](http://kubernetes.io) with reinterpretations of the `Reflector` and `Informer` abstractions from the go client.
+Rust client for [Kubernetes](http://kubernetes.io) in the style of [client-go](https://github.com/kubernetes/client-go). Contains rust reinterpretations of the `Reflector` and `Informer` abstractions (but without all the factories) to allow writing kubernetes controllers/operators easily.
 
-This client aims cater to the more common controller/operator case, but allows you sticking in dependencies like [k8s-openapi](https://github.com/Arnavion/k8s-openapi) for accurate struct representations.
+This client caters to the more common controller/operator case, but allows you to compile with the `openapi` feature to get automatic objects using the more accurate struct representations from [k8s-openapi](https://github.com/Arnavion/k8s-openapi).
 
 ## Usage
 See the [examples directory](./examples) for how to watch over resources in a simplistic way.
@@ -15,6 +15,21 @@ See [controller-rs](https://github.com/clux/controller-rs) for a full example wi
 
 **[API Docs](https://clux.github.io/kube-rs/kube/)**
 
+## Typed Api
+It's recommended to compile with the "openapi" feature if you want accurate native object structs.
+
+```rust
+//TODO: (see examples for now)
+```
+
+## Raw Api
+It's completely fine to not depend on `k8s-openapi` if you only are working with CRDs or you are happy to supply partial definitions of the native objects you are working with:
+
+```rust
+//TODO: (see examples for now)
+```
+
+
 ## Reflector
 One of the main abstractions exposed from `kube::api` is `Reflector<P, U>`. This is a cache of a resource that's meant to "reflect the resource state in etcd".
 

examples/crd_openapi.rs

@@ -1,5 +1,6 @@
 #[macro_use] extern crate log;
 #[macro_use] extern crate serde_derive;
+use either::Either::{Left, Right};
 use serde_json::json;
 
 use kube::{
@@ -31,12 +32,16 @@ fn main() -> Result<(), failure::Error> {
 
     // Delete any old versions of it first:
     let dp = DeleteParams::default();
-    if let Ok(res) = crds.delete("foos.clux.dev", &dp) {
-        info!("Deleted {}: ({:?})", res.metadata.name,
-            res.status.unwrap().conditions.unwrap().last());
+    crds.delete("foos.clux.dev", &dp)?.map_left(|o| {
+        info!("Deleted {}: ({:?})", o.metadata.name,
+            o.status.unwrap().conditions.unwrap().last());
+        // even PropagationPolicy::Foreground doesn't cause us to block here
+        // need to wait for it to actually go away
         std::thread::sleep(std::time::Duration::from_millis(1000));
-        // even PropagationPolicy::Foreground doesn't seem to block here..
-    }
+    }).map_right(|s| {
+        // it's gone.
+        info!("Deleted foos.clux.dev: ({:?})", s);
+    });
 
     // Create the CRD so we can create Foos in kube
     let foocrd = json!({
@@ -97,10 +102,6 @@ fn main() -> Result<(), failure::Error> {
     let f1cpy = foos.get("baz")?;
     assert_eq!(f1cpy.spec.info, "old baz");
 
-    // Delete it
-    let f1del = foos.delete("baz", &dp)?;
-    assert_eq!(f1del.spec.info, "old baz");
-
     // Replace its spec
     info!("Replace Foo baz");
     let foo_replace = json!({
@@ -118,6 +119,11 @@ fn main() -> Result<(), failure::Error> {
     assert_eq!(f1_replaced.spec.info, "new baz");
     assert!(f1_replaced.status.is_none());
 
+    // Delete it
+    foos.delete("baz", &dp)?.map_left(|f1del| {
+        assert_eq!(f1del.spec.info, "old baz");
+    });
+
 
     // Create Foo qux with status
     info!("Create Foo instance qux");
@@ -153,15 +159,23 @@ fn main() -> Result<(), failure::Error> {
     assert_eq!(o.spec.info, "patched qux");
     assert_eq!(o.spec.name, "qux"); // didn't blat existing params
 
-    // Check we have too instances
+    // Check we have 1 remaining instance
     let lp = ListParams::default();
     let res = foos.list(&lp)?;
-    assert_eq!(res.items.len(), 2);
+    assert_eq!(res.items.len(), 1);
 
-    // Cleanup the full colleciton
-    let res = foos.delete_collection(&lp)?;
-    let deleted = res.items.into_iter().map(|i| i.metadata.name).collect::<Vec<_>>();
-    info!("Deleted collection of foos: {:?}", deleted);
+    // Delete the last - expect a status back (instant delete)
+    assert!(foos.delete("qux", &dp)?.is_right());
 
+    // Cleanup the full collection - expect a wait
+    match foos.delete_collection(&lp)? {
+        Left(list) => {
+            let deleted = list.items.into_iter().map(|i| i.metadata.name).collect::<Vec<_>>();
+            info!("Deleted collection of foos: {:?}", deleted);
+        },
+        Right(status) => {
+            info!("Deleted collection of crds: status={:?}", status);
+        }
+    }
     Ok(())
 }

src/api/typed.rs

@@ -1,5 +1,6 @@
 #![allow(non_snake_case)]
 
+use either::{Either};
 use serde::de::DeserializeOwned;
 use std::marker::PhantomData;
 
@@ -14,6 +15,7 @@ use crate::api::resource::{
 };
 use crate::client::{
     APIClient,
+    ApiStatus,
 };
 use crate::{Result};
 
@@ -109,19 +111,17 @@ impl<P, U> OpenApi<P, U> where
         let req = self.api.create(&pp, data)?;
         self.client.request::<Object<P, U>>(req)
     }
-    // TODO: fix return type
-    pub fn delete(&self, name: &str, dp: &DeleteParams) -> Result<Object<P, U>> {
+    pub fn delete(&self, name: &str, dp: &DeleteParams) -> Result<Either<Object<P, U>, ApiStatus>> {
         let req = self.api.delete(name, &dp)?;
-        self.client.request::<Object<P, U>>(req)
+        self.client.request_status::<Object<P, U>>(req)
     }
     pub fn list(&self, lp: &ListParams) -> Result<ObjectList<Object<P, U>>> {
         let req = self.api.list(&lp)?;
         self.client.request::<ObjectList<Object<P, U>>>(req)
     }
-    // TODO: fix return type
-    pub fn delete_collection(&self, lp: &ListParams) -> Result<ObjectList<Object<P, U>>> {
+    pub fn delete_collection(&self, lp: &ListParams) -> Result<Either<ObjectList<Object<P, U>>, ApiStatus>> {
         let req = self.api.delete_collection(&lp)?;
-        self.client.request::<ObjectList<Object<P, U>>>(req)
+        self.client.request_status::<ObjectList<Object<P, U>>>(req)
     }
     pub fn patch(&self, name: &str, pp: &PostParams, patch: Vec<u8>) -> Result<Object<P, U>> {
         let req = self.api.patch(name, &pp, patch)?;

src/client/mod.rs

@@ -1,5 +1,9 @@
 //! A basic API client with standard kube error handling
 
+use serde_json::Value;
+use either::{Right, Left};
+use either::Either;
+use http::StatusCode;
 use http;
 use serde::de::DeserializeOwned;
 use serde_json;
@@ -8,7 +12,8 @@ use crate::{ApiError, Error, ErrorKind, Result};
 use crate::config::Configuration;
 
 
-#[derive(Deserialize)]
+#[allow(non_snake_case)]
+#[derive(Deserialize, Debug)]
 pub struct StatusDetails {
     #[serde(default, skip_serializing_if = "String::is_empty")]
     pub name: String,
@@ -24,7 +29,7 @@ pub struct StatusDetails {
     pub retryAfterSeconds: u32
 }
 
-#[derive(Deserialize)]
+#[derive(Deserialize, Debug)]
 pub struct StatusCause {
     #[serde(default, skip_serializing_if = "String::is_empty")]
     pub reason: String,
@@ -34,7 +39,7 @@ pub struct StatusCause {
     pub field: String,
 }
 
-#[derive(Deserialize)]
+#[derive(Deserialize, Debug)]
 pub struct ApiStatus {
     // TODO: typemeta
     // TODO: metadata that can be completely empty (listmeta...)
@@ -44,16 +49,12 @@ pub struct ApiStatus {
     pub message: String,
     #[serde(default, skip_serializing_if = "String::is_empty")]
     pub reason: String,
-
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub details: Option<StatusDetails>,
-
     #[serde(default, skip_serializing_if = "num::Zero::is_zero")]
     pub code: u16,
 }
 
-
-
 /// APIClient requires `config::Configuration` includes client to connect with kubernetes cluster.
 #[derive(Clone)]
 pub struct APIClient {
@@ -93,33 +94,38 @@ impl APIClient {
         //trace!("Response Headers: {:?}", res.headers());
         let s = res.status();
         let text = res.text().context(ErrorKind::RequestParse)?;
-        match res.error_for_status() {
-            Err(e) => {
-                // Print better debug when things do fail
-                if let Ok(errdata) = serde_json::from_str::<ApiError>(&text) {
-                    debug!("Unsuccessful: {:?}", errdata);
-                    Err(ErrorKind::Api(errdata))?;
-                } else {
-                    // In case some parts of ApiError for some reason don't exist..
-                    error!("Unsuccessful data error parse: {}", text);
-                    Err(ErrorKind::SerdeParse)?; // should not happen
-                }
-                // Propagate errors properly via reqwest
-                let ae = ApiError {
-                    status: s.to_string(),
-                    code: s.as_u16(),
-                    message: format!("{:?}", e),
-                    reason: format!("{}", e),
-                };
-                debug!("Unsuccessful: {:?} (reconstruct)", ae);
-                Err(ErrorKind::Api(ae))?
-            },
-            Ok(_res) => {
-                serde_json::from_str(&text).map_err(|e| {
-                    warn!("{}, {:?}", text, e);
-                    Error::from(ErrorKind::SerdeParse)
-                })
-            }
+        res.error_for_status().map_err(|e| make_api_error(&text, e, &s))?;
+
+        serde_json::from_str(&text).map_err(|e| {
+            warn!("{}, {:?}", text, e);
+            Error::from(ErrorKind::SerdeParse)
+        })
+    }
+
+    pub fn request_status<T>(&self, request: http::Request<Vec<u8>>) -> Result<Either<T, ApiStatus>>
+    where
+        T: DeserializeOwned,
+    {
+        let mut res : reqwest::Response = self.send(request)?;
+        trace!("{} {}", res.status().as_str(), res.url());
+        //trace!("Response Headers: {:?}", res.headers());
+        let s = res.status();
+        let text = res.text().context(ErrorKind::RequestParse)?;
+        res.error_for_status().map_err(|e| make_api_error(&text, e, &s))?;
+
+        // It needs to be JSON:
+        let v: Value = serde_json::from_str(&text).context(ErrorKind::SerdeParse)?;;
+        if v["kind"] == "Status" {
+            trace!("ApiStatus from {}", text);
+            Ok(Right(serde_json::from_str::<ApiStatus>(&text).map_err(|e| {
+                warn!("{}, {:?}", text, e);
+                Error::from(ErrorKind::SerdeParse)
+            })?))
+        } else {
+            Ok(Left(serde_json::from_str::<T>(&text).map_err(|e| {
+                warn!("{}, {:?}", text, e);
+                Error::from(ErrorKind::SerdeParse)
+            })?))
         }
     }
 
@@ -132,39 +138,44 @@ impl APIClient {
         //trace!("Response Headers: {:?}", res.headers());
         let s = res.status();
         let text = res.text().context(ErrorKind::RequestParse)?;
-        match res.error_for_status() {
-            Err(e) => {
-                // Print better debug when things do fail
-                if let Ok(errdata) = serde_json::from_str::<ApiError>(&text) {
-                    debug!("Unsuccessful: {:?}", errdata);
-                    Err(ErrorKind::Api(errdata))?;
-                } else {
-                    // In case some parts of ApiError for some reason don't exist..
-                    error!("Unsuccessful data error parse: {}", text);
-                    Err(ErrorKind::SerdeParse)?; // should not happen
-                }
-                // Propagate errors properly via reqwest
-                let ae = ApiError {
-                    status: s.to_string(),
-                    code: s.as_u16(),
-                    message: format!("{:?}", e),
-                    reason: format!("{}", e),
-                };
-                debug!("Unsuccessful: {:?} (reconstruct)", ae);
-                Err(ErrorKind::Api(ae))?
-            },
-            Ok(_res) => {
-                // Should be able to coerce result into Vec<T> at this point
-                let mut xs : Vec<T> = vec![];
-                for l in text.lines() {
-                    let r = serde_json::from_str(&l).map_err(|e| {
-                        warn!("{} {:?}", l, e);
-                        Error::from(ErrorKind::SerdeParse)
-                    })?;
-                    xs.push(r);
-                }
-                Ok(xs)
-            },
+        res.error_for_status().map_err(|e| make_api_error(&text, e, &s))?;
+
+        // Should be able to coerce result into Vec<T> at this point
+        let mut xs : Vec<T> = vec![];
+        for l in text.lines() {
+            let r = serde_json::from_str(&l).map_err(|e| {
+                warn!("{} {:?}", l, e);
+                Error::from(ErrorKind::SerdeParse)
+            })?;
+            xs.push(r);
         }
+        Ok(xs)
+    }
+}
+
+/// Kubernetes returned error handling
+///
+/// Either kube returned an explicit ApiError struct,
+/// or it someohow returned something we couldn't parse as one.
+///
+/// In either case, present an ApiError upstream.
+/// The latter is probably a bug if encountered.
+fn make_api_error(text: &str, error: reqwest::Error, s: &StatusCode) -> ErrorKind {
+    // Print better debug when things do fail
+    //trace!("Parsing error: {}", text);
+    if let Ok(errdata) = serde_json::from_str::<ApiError>(text) {
+        debug!("Unsuccessful: {:?}", errdata);
+        ErrorKind::Api(errdata)
+    } else {
+        warn!("Unsuccessful data error parse: {}", text);
+        // Propagate errors properly via reqwest
+        let ae = ApiError {
+            status: s.to_string(),
+            code: s.as_u16(),
+            message: format!("{:?}", error),
+            reason: format!("{}", error),
+        };
+        debug!("Unsuccessful: {:?} (reconstruct)", ae);
+        ErrorKind::Api(ae)
     }
 }