Automatic merge from submit-queue
Fix ensureDnsRecords comments for federated services
I went to look at the source comments, because the documentation is not exhaustive about what kind of DNS records are created for federated services (and http://blog.kubernetes.io/2016/07/cross-cluster-services.html is wrong...).
It turns out that even the comment is not in sync with the code: two out of three records listed use `.federation`, while the author probably meant `.mydomain.com` (which has less chance of getting mixed up with `myfed`). I fixed those, as well as a few spelling and parenthesis errors. Hopefully this will help others save time and not scratch their heads.
cc @quinton-hoole
Automatic merge from submit-queue (batch tested with PRs 44469, 44566, 44467, 44526)
[Federation]Fix panic: index out of range
When the number of clusterStatusNew's Conditions is different from
the number of clusterStatusOld's Conditions, clustercontroller
will panic. We should check it before comparing.
When the number of clusterStatusNew's Conditions is different from
the number of clusterStatusOld's Conditions, clustercontroller
will panic. We should check it before comparing.
I went to look at the source comments, because the documentation is not exhaustive about what kind of DNS records are created for federated services (and http://blog.kubernetes.io/2016/07/cross-cluster-services.html is wrong...).
It turns out that even the comment is not in sync with the code: two out of three records listed use `.federation`, while the author probably meant `.mydomain.com` (which has less chance of getting mixed up with `myfed`). I fixed those, as well as a few spelling and parenthesis errors. Hopefully this will help others save time and not scratch their heads.
This change uses an adapter class to abstracts the interaction of the
secret controller with the secret api type. This is the first step to
creating a generic controller that can target any type for which an
adapter exists.
Automatic merge from submit-queue
[Federation] Add integration test for secrets
This PR adds an integration test for secrets that:
- performs create/read/update/delete on federation resources and validates that the changes are propagated to member clusters.
- uses an abstraction layer (fixture and adapter) to minimize the code required to support each federated type
- It should be possible to replace a test-specific adapter with a runtime adapter in the future (as per #41050)
- reuses fixture (federation api and clusters) across different resource types to minimize setup overhead
- on a fast machine, setup takes ~4s, and validating each type takes ~2s
- uses the [Subtest feature added in Go 1.7](https://blog.golang.org/subtests) to allow the test for a specific controller to be run in isolation
- ``make test-integration WHAT="federation -test.run=TestFederationCRUD/secret"``
Once this PR merges the test can be extended to target other federated types.
This PR targets #40705
cc: @kubernetes/sig-federation-pr-reviews @derekwaynecarr
Automatic merge from submit-queue
[Federation] Use cascading deletion when deleting resources in underlying clusters
The Federation control plane issues a delete command unless it wants to orphan the underlying per-cluster resource. When issuing that command, always set the orphanDependents to false.
/release-note-none
/sig-federation
Automatic merge from submit-queue (batch tested with PRs 44084, 42964)
Removing both finalizers in federation controllers in a single update
Fixes https://github.com/kubernetes/kubernetes/issues/43828
There is a bug right now where the controller fails to delete the object if one finalizer is removed and the second isnt.
This updates the code so that both the finalizers are removed in a single API call. Kept the code changes minimum to enable cherrypick in 1.6.x
cc @csbell @kubernetes/sig-federation-bugs
The Federation control plane issues a delete command unless it wants to orphan the underlying per-cluster resource. When issuing that command, always set the orphanDependents to false.
Automatic merge from submit-queue (batch tested with PRs 41297, 42638, 42666, 43039, 42567)
Delete offline restclient from clusterKubeClientMap
When federation controller manager checks cluster status, it will
delete the offline cluster from clusterSet, but do not delete the
corresponding restclient from the map clusterKubeClientMap for
the offline cluster. This patch can fix it.
**What this PR does / why we need it**:
**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #
**Special notes for your reviewer**:
**Release note**:
```release-note
```
Automatic merge from submit-queue
Update the description to match function
The description of NewReplicaSetController() does not match
its function, and the description of NewDeploymentController()
does not match its function. Let's update their descriptions.
**What this PR does / why we need it**:
**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #
**Special notes for your reviewer**:
**Release note**:
```release-note
```
Automatic merge from submit-queue
[Federation] Fix deletion logic in service controller
This is a regression from 1.5 exposed by cascading deletions. In order to apply updates, the service controller locks access to a cached service and spawns go routines without waiting for them. When updates and deletions arrive in quick succession, previous goroutines remain active and race with the deletion logic. Coupled with this, the service_helper was not re-evaluating the value of the DeletionTimestamp.
Without this patch, federation will sometimes leak resources at destruction time about half the time.
In e2e land, about 4-5 test runs cause service tests to eat up all global fwd-ing rules and in turn, every subsequent ingress test will fail until we manually clean up leaked resources. No possibility to go green in fed e2e until this is merged.
This is a regression from 1.5 exposed by cascading deltions. In order to apply updates, the service controller locks access to a cached service and spawns go routines without waiting for them. When updates and deletions arrive in quick succession, previous goroutines remain active and race with the deletion logic. Coupled with this, the service_helper was not re-evaluating the value of the DeletionTimestamp.
Without this patch, federation will sometimes leak resources at destruction time.
When federation controller manager checks cluster status, it will
delete the offline cluster from clusterSet, but do not delete the
corresponding restclient from the map clusterKubeClientMap for
the offline cluster. This patch can fix it.
The unit test for the ingress controller was previously adding
a cluster twice, which resulted in a cluster being deleted and added
back. The deletion was racing the controller shutdown to close
informer channels. This change ensures that the informer clears its
map of informers when Stop() is called to prevent a double close, and
that the test no longer adds the cluster twice.
The Deployment controller was not propagating ReadyReplicas to underlying clusters causing these errors:
```
Error syncing cluster controller: Deployment.apps "federation-deployment" is invalid: status.availableReplicas: Invalid value: 5: cannot be greater than readyReplicas
```
This was caught in e2e testing and is a 1.6 regression for support that was added in #37959. Without this fix, users will be unable to scale up their deployments.
The description of NewReplicaSetController() does not match
its function, and the description of NewDeploymentController()
does not match its function. Let's update their descriptions.
Automatic merge from submit-queue
Add ProviderUid support to Federated Ingress
This PR (along with GLBC support [here](https://github.com/kubernetes/ingress/pull/278)) is a proposed fix for #39989. The Ingress controller uses a configMap reconciliation process to ensure that all underlying ingresses agree on a unique UID. This works for all of GLBC's resources except firewalls which need their own cluster-unique UID. This PR introduces a ProviderUid which is maintained and synchronized cross-cluster much like the UID. We chose to derive the ProviderUid from the cluster name (via md5 hash).
Testing here is augmented to guarantee that configMaps are adequately propagated prior to Ingress creation.
```release-note
Federated Ingress over GCE no longer requires separate firewall rules to be created for each cluster to circumvent flapping firewall health checks.
```
cc @madhusudancs @quinton-hoole
Automatic merge from submit-queue (batch tested with PRs 42369, 42375, 42397, 42435, 42455)
Fixing federation controllers to support controllers flag
Fixes https://github.com/kubernetes/kubernetes/issues/42374
cc @kubernetes/sig-federation-pr-reviews
gives each ingress object a cluster-unique Uid that can be
leveraged by ingress providers.
In the process, supplement the testing of configMap updates to
ensure that the updates are propagated prior to any ingress
object being created. Configmap key/vals for Uid and ProviderUid
must exist at time of Ingress creation.
Automatic merge from submit-queue (batch tested with PRs 41931, 39821, 41841, 42197, 42195)
Revert conditional updates to configmap that breaks ingress
Don't prevent configmap updates to happen on existing ingress-uid configmap despite the lack of ingress objects. Otherwise, ingress objects get created with the wrong name.
Automatic merge from submit-queue (batch tested with PRs 42044, 41694, 41927, 42050, 41987)
federation: use generated listers
**What this PR does / why we need it**: switch federation code to use generated listers
**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #
**Special notes for your reviewer**:
**Release note**:
```release-note
```
cc @smarterclayton @derekwaynecarr @marun @gmarek @wojtek-t @deads2k @liggitt @sttts @timothysc @nikhiljindal @jianhuiz @kubernetes/sig-federation-pr-reviews @kubernetes/sig-scalability-pr-reviews
Automatic merge from submit-queue
error strings should not end with punctuation
**What this PR does / why we need it**:
Delete the end punctuation of error strings
**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #
**Special notes for your reviewer**:
https://github.com/golang/go/wiki/CodeReviewComments#error-strings
**Release note**:
```release-note
```
Signed-off-by: yupeng <yu.peng36@zte.com.cn>
These files have been created lately, so we don't have much information
about them anyway, so let's just:
- Remove assignees and make them approvers
- Copy approves as reviewers
Automatic merge from submit-queue (batch tested with PRs 39475, 38666, 39327, 38396, 39613)
Fix a bug in cascading deletion of federation objects
When FinalizerOrphan is present and set to true in federated object we are currently removing the FinalizerOrphan first and then removing FinalizerDeleteFromUnderlyingClusters. if a reconciliation is triggered in between the finalizer removals, it has undesired effect of object deletion in federated cluster.
So we should remove FinalizerDeleteFromUnderlyingClusters first and then remove FinalizerOrphan, when FinalizerOrphan is set to true.
@nikhiljindal, @madhusudancs
Automatic merge from submit-queue (batch tested with PRs 38212, 38792, 39641, 36390, 39005)
Updating federated service controller to support cascading deletion
Ref https://github.com/kubernetes/kubernetes/issues/33612
Service controller is special than other federation controllers because it does not use federatedinformer and updater to sync services (it was written before we had those frameworks).
Updating service controller code to instantiate these frameworks and then use deletion helper to perform cascading deletion.
Note that, I havent changed the queuing logic in this PR so we still dont use federated informer to manage the queue. Will do that in the next PR.
cc @kubernetes/sig-federation-misc @mwielgus @quinton-hoole
```release-note
federation: Adding support for DeleteOptions.OrphanDependents for federated services. Setting it to false while deleting a federated service also deletes the corresponding services from all registered clusters.
```
Automatic merge from submit-queue
Enable update tests in federated namespace controller
Previously it was disabled because of flakiness.
cc: @nikhiljindal @madhusudancs
Automatic merge from submit-queue
Fix typo for federation/*
**What this PR does / why we need it**:
Increase code readability for this new member in v1.5
**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*:
**Special notes for your reviewer**:
Could we develop a typo-fix bot along with a k8s terminology dictionary ?
**Release note**:
```release-note
```
Automatic merge from submit-queue
Add CoreDNS as Federation DNS provider
This PR contains following
1. DNS provider implementation for CoreDNS with etcd as backend
2. Script for optionally deploying CoreDNS in federation control plane
Still to do:
1. HA for CoreDNS service need to be handled, would be taken up in subsequent PR
2. Need to configure CoreDNS as fallback DNS to in-cluster KubeDNS(Manual Step). Need to consider automation.
Request for review: @quinton-hoole @madhusudancs @nikhiljindal @kshafiee @deepak-vij
cc @kubernetes/sig-cluster-federation
Automatic merge from submit-queue
Enable updates in federated ingress controller tests
And make channel in test helper non-blocking.
cc: @nikhiljindal @madhusudancs
Automatic merge from submit-queue
Fix package aliases to follow golang convention
Some package aliases are not not align with golang convention https://blog.golang.org/package-names. This PR fixes them. Also adds a verify script and presubmit checks.
Fixes#35070.
cc/ @timstclair @Random-Liu
Automatic merge from submit-queue
Fix infinte loop in federated ingress controller
Previously ingress controller was constantly scheduling reconcilation, even if no updates were needed. That behavior creates a big mess in the logs and consumes resources.
This pr also fixes the stop function for federated ingress controller.
cc: @nikhiljindal @madhusudancs
Automatic merge from submit-queue
Revision handling in federated deployment controller
Deployment controller in regular kubernetes automatically adds an annotation in deployment. This causes a bit of confusion in controller and tests. This PR skips revision annotation in checks. In the next K8S release we will need to have better support for deployment revisions.
Helps with #36588
cc: @nikhiljindal @madhusudancs
Automatic merge from submit-queue
Stop deleting underlying services when federation service is deleted
Fixes https://github.com/kubernetes/kubernetes/issues/36799
Fixing federation service controller to not delete services from underlying clusters when federated service is deleted.
None of the federation controller should do this unless explicitly asked by the user using DeleteOptions. This is the only federation controller that does that.
cc @kubernetes/sig-cluster-federation @madhusudancs
```release-note
federation service controller: stop deleting services from underlying clusters when federated service is deleted.
```
Automatic merge from submit-queue
Fixing the logic to select first cluster in federated ingress controller
Ref https://github.com/kubernetes/kubernetes/issues/36074.
Before this change, ingress controller was using cluster with clusterIndex = 0 as the first cluster to create the ingress in.
But the ordering of clusters can change and hence ingress controller ended up creating the ingress in multiple clusters.
This PR fixes it by using an annotation on federated ingress. Controller now picks up a cluster randomly as the first cluster and creates ingress there. This clusters name is stored as an annotation on the federated ingress. Contoller does not create an ingress in any other cluster if this annotation is set on the federated ingress and IP has not been propagated. Once IP has been propagated, controller creates the ingress in all clusters.
cc @kubernetes/sig-cluster-federation @madhusudancs
Automatic merge from submit-queue
populate ready replicas and aviable replicas to federated replicaset …
populate ready replicas and aviable replicas to federated replicaset status
@nikhiljindal #33312#29481#32771
@deepak-vij
Automatic merge from submit-queue
Fixing the ingress annotation key in federated ingress controller
Fixing a few issues found during debugging https://github.com/kubernetes/kubernetes/issues/36074.
cc @kubernetes/sig-cluster-federation @madhusudancs @bprashanth
Automatic merge from submit-queue
Make desired objects in fed controller reconcilation function fully writable
When creating desired objects for sub-cluster in reconciliate functions we don't make full copies. This may result in unexpected race condition in the future when someone actually adds some object modifications (not needed in most cases but anyway - the cost of extra copy will be relatively small comparing to inter-cluster traffic). In case of ReplicaSet and Deployment the spec copy was a shallow one - ok for setting just replicas but also error-prone in the future.
cc: @quinton-hoole @nikhiljindal @madhusudancs
