Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

Infinite reconciliation loop (rawstatuscollection) #1380

Merged
merged 1 commit into from Mar 25, 2021
Merged

Infinite reconciliation loop (rawstatuscollection) #1380

merged 1 commit into from Mar 25, 2021

Conversation

cebernardi
Copy link
Contributor

What this PR does / why we need it:
Federated resources that have target resources without status field are constantly evaluated as "changed", if RawStatusCollection is Enabled. This triggers an infinite loop when multiple FederatedResources are created at the same time in the cluster.
Also, this PR applies to collectedStatuses the same transformation that is applied to the FederatedResource status (json.Marshal - json.Unmarshall), so to prevent avoidable changes (and reconciliations) to the FederatedResources.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1373

Special notes for your reviewer:

@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Mar 17, 2021
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 17, 2021
@hectorj2f
Copy link
Contributor

@cebernardi Did you manage to figure it out the issue with the tests ?

@cebernardi
Copy link
Contributor Author

@cebernardi Did you manage to figure it out the issue with the tests ?

tests are failing because of

1.15.3: Pulling from library/golang
 library/golang
docker: toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit.

I'm trying to retrigger the build..

@hectorj2f
Copy link
Contributor

@cebernardi I can do that for you.

@cebernardi
Copy link
Contributor Author

@cebernardi I can do that for you.

yes please @hectorj2f ...
It just failed again for the same reason...

@hectorj2f
Copy link
Contributor

@cebernardi Done, we now need you to sign the cla.

@cebernardi
Copy link
Contributor Author

cebernardi commented Mar 19, 2021

@hectorj2f let me check with my company, they should have already signed it, and they might be in the process of returning it.. will let you know asap

@cebernardi
Copy link
Contributor Author

@hectorj2f the CLA is signed.. is there a way to trigger again the check?

@hectorj2f
Copy link
Contributor

@cebernardi No, I don't know it. Perhaps close-and-reopen the PR. I am not sure.

@cebernardi
Copy link
Contributor Author

/check-cla

@cebernardi cebernardi closed this Mar 22, 2021
@cebernardi cebernardi reopened this Mar 22, 2021
@cebernardi cebernardi closed this Mar 22, 2021
@cebernardi cebernardi reopened this Mar 22, 2021
@cebernardi
Copy link
Contributor Author

double checked again.. CLA was signed BUT not returned to Linux Foundation..

@hectorj2f
Copy link
Contributor

/check-cla

@swiftslee
Copy link
Contributor

Too many commits. I think you need to merge the commits into one.

@hectorj2f
Copy link
Contributor

@cebernardi i tested your changes and they work. We need a proper commit log, and address your signing issues.

@hectorj2f
Copy link
Contributor

/check-cla

applied json marshalling to collectedRemoteStatus to make it comparable with remoteStatus on the federated resource
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Mar 24, 2021
Copy link
Contributor

@hectorj2f hectorj2f left a comment

Choose a reason for hiding this comment

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

/approve

@hectorj2f hectorj2f requested a review from makkes March 24, 2021 15:11
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cebernardi, hectorj2f

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 24, 2021
@hectorj2f
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 25, 2021
@k8s-ci-robot k8s-ci-robot merged commit e13973c into kubernetes-retired:master Mar 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Intermittent huge selective delay in propagation of federated resources and infinite reconciliation loop
4 participants