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

impossible to write timestamps to status objects without causing controller to loop #354

Closed
clux opened this issue Dec 23, 2020 · 4 comments
Labels
bug Something isn't working runtime controller runtime related

Comments

@clux
Copy link
Member

clux commented Dec 23, 2020

A small diff to a controller like:

diff --git src/manager.rs src/manager.rs
index 544b495..277f25c 100644
--- src/manager.rs
+++ src/manager.rs
@@ -26,9 +26,10 @@ pub struct FooSpec {
     info: String,
 }
 
-#[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)]
+#[derive(Deserialize, Serialize, Clone, Debug, JsonSchema)]
 pub struct FooStatus {
     is_bad: bool,
+    last_updated: Option<DateTime<Utc>>,
 }
 
 // Context for our reconciler
@@ -54,6 +55,7 @@ async fn reconcile(foo: Foo, ctx: Context<Data>) -> Result<ReconcilerAction, Err
     let new_status = serde_json::to_vec(&json!({
         "status": FooStatus {
             is_bad: foo.spec.info.contains("bad"),
+            last_updated: Some(Utc::now()),
         }
     }))
     .context(SerializationFailed)?;

Causes the controller to spin repeatedly:

Dec 23 13:46:51.991  INFO controller::manager: Reconciled (ObjectRef { kind: (), name: "bad", namespace: Some("default") }, ReconcilerAction { requeue_after: Some(1800s) })
Dec 23 13:46:51.997  INFO controller::manager: Reconciled (ObjectRef { kind: (), name: "bad", namespace: Some("default") }, ReconcilerAction { requeue_after: Some(1800s) })
Dec 23 13:46:52.003  INFO controller::manager: Reconciled (ObjectRef { kind: (), name: "bad", namespace: Some("default") }, ReconcilerAction { requeue_after: Some(1800s) })
Dec 23 13:46:52.008  INFO controller::manager: Reconciled (ObjectRef { kind: (), name: "bad", namespace: Some("default") }, ReconcilerAction { requeue_after: Some(1800s) })
Dec 23 13:46:52.014  INFO controller::manager: Reconciled (ObjectRef { kind: (), name: "bad", namespace: Some("default") }, ReconcilerAction { requeue_after: Some(1800s) })
Dec 23 13:46:52.020  INFO controller::manager: Reconciled (ObjectRef { kind: (), name: "bad", namespace: Some("default") }, ReconcilerAction { requeue_after: Some(1800s) })
Dec 23 13:46:52.027  INFO controller::manager: Reconciled (ObjectRef { kind: (), name: "bad", namespace: Some("default") }, ReconcilerAction { requeue_after: Some(1800s) })
Dec 23 13:46:52.032  INFO controller::manager: Reconciled (ObjectRef { kind: (), name: "bad", namespace: Some("default") }, ReconcilerAction { requeue_after: Some(1800s) })
Dec 23 13:46:52.037  INFO controller::manager: Reconciled (ObjectRef { kind: (), name: "bad", namespace: Some("default") }, ReconcilerAction { requeue_after: Some(1800s) })
Dec 23 13:46:52.056  INFO controller::manager: Reconciled (ObjectRef { kind: (), name: "bad", namespace: Some("default") }, ReconcilerAction { requeue_after: Some(1800s) })
Dec 23 13:46:52.074  INFO controller::manager: Reconciled (ObjectRef { kind: (), name: "bad", namespace: Some("default") }, ReconcilerAction { requeue_after: Some(1800s) })
Dec 23 13:46:52.086  INFO controller::manager: Reconciled (ObjectRef { kind: (), name: "bad", namespace: Some("default") }, ReconcilerAction { requeue_after: Some(1800s) })
Dec 23 13:46:52.100  INFO controller::manager: Reconciled (ObjectRef { kind: (), name: "bad", namespace: Some("default") }, ReconcilerAction { requeue_after: Some(1800s) })
Dec 23 13:46:52.113  INFO controller::manager: Reconciled (ObjectRef { kind: (), name: "bad", namespace: Some("default") }, ReconcilerAction { requeue_after: Some(1800s) })

Related to #318 and #279. It was slightly annoying then, but now it's actually preventing things, so we have to tackle it one way or another.

@clux clux added bug Something isn't working runtime controller runtime related labels Dec 23, 2020
@nightkr
Copy link
Member

nightkr commented Dec 23, 2020

On one hand, it could be argued that the client shouldn't send no-op patches to the apiserver anyway. On the other, doing that kind of diffing locally could be pretty error-prone..

Then again, any local filtering mechanism would need to implement the same logic anyway.

@clux
Copy link
Member Author

clux commented Dec 23, 2020

On one hand, it could be argued that the client shouldn't send no-op patches to the apiserver anyway. On the other, doing that kind of diffing locally could be pretty error-prone..

Oh, that is a good point. Yeah, this probably isn't a bug.
Sorry, just second guessing everything before the release 😅

@clux clux closed this as completed Dec 23, 2020
@clux
Copy link
Member Author

clux commented Dec 23, 2020

hm, then again, doing a noop update to the server is kind of useful to a controller to indicate that work has been done and the thing isn't dead...

EDIT: particularly if the only thing changes was the status object. It's a bit difficult to check every single status property before writing our current one in a reconciler. At the very least it will cause very hairy reconcilers.

@clux clux reopened this Dec 23, 2020
clux added a commit to kube-rs/controller-rs that referenced this issue Dec 23, 2020
also not including timestamps yet due to kube-rs/kube#354
@clux
Copy link
Member Author

clux commented Dec 31, 2020

Closing in favour of #279 - will explain there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working runtime controller runtime related
Projects
None yet
Development

No branches or pull requests

2 participants