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

support for deletion timestamp added, tests failed #574

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Catoverflow
Copy link
Collaborator

@Catoverflow Catoverflow commented Jan 13, 2025

This PR implemented the model & executable part for
https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/replicaset/replica_set.go#L721-L723

Test is still broken, working on fixing that

@@ -120,6 +120,13 @@ pub fn reconcile_core(v_replica_set: &VReplicaSet, resp_o: Option<Response<VoidE
if pods_or_none.is_none() {
return (error_state(state), None);
}
if v_replica_set.metadata().has_deletion_timestamp() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move it to the Init step? If the VReplicaSet has deletion timestamp, then there is no need to list the pods whatsoever.

Copy link
Collaborator Author

@Catoverflow Catoverflow Jan 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is I cannot align well the code block in k8s controller with the step in anvil. Take the line of code I want to model as an example, I'm not sure if we filter out the entire func (rsc *ReplicaSetController) syncReplicaSet(ctx context.Context, key string) where should I put the corresponding timestamp checking logic in anvil's model. In other words, I don't know if k8s itself put this part in init or afterlistpod step.

Do you have any suggestion?

@marshtompsxd
Copy link
Collaborator

So the conformance proof passes but the liveness proof breaks, right?

@tianyin
Copy link
Member

tianyin commented Jan 14, 2025

@Catoverflow could you write some brief descriptions when you open PRs?

@Catoverflow
Copy link
Collaborator Author

Catoverflow commented Jan 14, 2025

So the conformance proof passes but the liveness proof breaks, right?

Yes, 2 failed tests are

error: assertion failed
   --> v2/controllers/vreplicaset_controller/proof/liveness/resource_match.rs:573:87
    |
573 |     assert forall |s, s_prime| pre(s) && #[trigger] stronger_next(s, s_prime) implies pre(s_prime) || post(s_prime) by {
    |                                                                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ assertion failed

note: function body check: not all errors may have been reported; rerun with a higher value for --multiple-errors to find other potential errors in this function
   --> v2/controllers/vreplicaset_controller/proof/liveness/resource_match.rs:460:1
    |
460 | / pub proof fn lemma_from_after_receive_list_pods_resp_to_send_create_pod_req(
461 | |     vrs: VReplicaSetView, spec: TempPred<ClusterState>, cluster: Cluster, controller_id: int,
462 | |     resp_msg: Message, diff: int
463 | | )
    | |_^

error: assertion failed
    --> v2/controllers/vreplicaset_controller/proof/liveness/resource_match.rs:1071:87
     |
1071 |     assert forall |s, s_prime| pre(s) && #[trigger] stronger_next(s, s_prime) implies pre(s_prime) || post(s_prime) by {
     |                                                                               

I'm trying to find a way to expand the pre and post wrappers to find the specific clauses

@Catoverflow
Copy link
Collaborator Author

@Catoverflow could you write some brief descriptions when you open PRs?

added

@marshtompsxd
Copy link
Collaborator

@Catoverflow You might need to revise the premise of the ESR property to focus on reasoning about VReplicaSet that has no deletion timestamp. You can work with @codyjrivera on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants