Repository: kubernetes-sigs/cluster-api
Pull request #3385 (2020-07-23 13:25:06)

🌱  Log when and why a machine is marked for remediation
**What this PR does / why we need it**:
Right now MHC logs almost nothing on the `-v=0` log level. This adds a new message when a machine fails a health check and we are marking it for remediation. We [store information](https://github.com/benmoss/cluster-api/blob/c510161e2c2639a4c5e8a1683c34a016a1d236ac/controllers/machinehealthcheck_targets.go#L94-L158) about why the health check fails on the condition, so we can use that to surface the failure now. 

Also removes the "patching machine" log line from healthy machines, this is almost always useless in my experience.


Patch:
diff --git a/controllers/machinehealthcheck_controller.go b/controllers/machinehealthcheck_controller.go
index 52cfcefe1..30d5038f6 100644
--- a/controllers/machinehealthcheck_controller.go
+++ b/controllers/machinehealthcheck_controller.go
@@ -250,7 +250,8 @@ func (r *MachineHealthCheckReconciler) reconcile(ctx context.Context, logger log
 	// mark for remediation
 	errList := []error{}
 	for _, t := range unhealthy {
-		logger.V(3).Info("Target meets unhealthy criteria, triggers remediation", "target", t.string())
+		condition := conditions.Get(t.Machine, clusterv1.MachineHealthCheckSuccededCondition)
+		logger.Info("Target has failed health check, marking for remediation", "target", t.string(), "reason", condition.Reason, "message", condition.Message)
 
 		conditions.MarkFalse(t.Machine, clusterv1.MachineOwnerRemediatedCondition, clusterv1.WaitingForRemediation, clusterv1.ConditionSeverityWarning, "MachineHealthCheck failed")
 		if err := t.patchHelper.Patch(ctx, t.Machine); err != nil {
@@ -265,7 +266,6 @@ func (r *MachineHealthCheckReconciler) reconcile(ctx context.Context, logger log
 		)
 	}
 	for _, t := range healthy {
-		logger.V(3).Info("patching machine", "machine", t.Machine.GetName())
 		if err := t.patchHelper.Patch(ctx, t.Machine); err != nil {
 			return ctrl.Result{}, errors.Wrapf(err, "Failed to patch healthy machine status for machine %q", t.Machine.Name)
 		}
diff --git a/docs/book/src/introduction.md b/docs/book/src/introduction.md
index 029d746df..40e45e713 100644
--- a/docs/book/src/introduction.md
+++ b/docs/book/src/introduction.md
@@ -1,32 +1,16 @@
-# The Cluster API project<div style="float: right; position: relative; display: inline;"><img src="images/introduction.png" width="160px" /></div>
+# The Cluster API Book <div style="float: right; position: relative; display: inline;"><img src="images/introduction.png" width="160px" /></div>
 
-Cluster API is a Kubernetes sub-project focused on providing declarative APIs and tooling to simplify provisioning, upgrading, and operating multiple Kubernetes clusters.
+**Note:** Impatient readers may head straight to [Quick
+Start](./user/quick-start.md).
 
-Started by the Kubernetes Special Interest Group (SIG) Cluster Lifecycle, the Cluster API project uses Kubernetes-style APIs and patterns to automate cluster lifecycle management for platform operators. The supporting infrastructure, like virtual machines, networks, load balancers, and VPCs, as well as the Kubernetes cluster configuration are all defined in the same way that application developers operate deploying and managing their workloads. This enables consistent and repeatable cluster deployments across a wide variety of infrastructure environments.
+**Using Cluster API v1alpha2? Check the [legacy
+documentation](https://release-0-2.cluster-api.sigs.k8s.io)**
 
-### Getting started
-* [Quick start](./user/quick-start.md)
-* [Concepts](./user/concepts.md)
-* [Developer guide](./developer/guide.md)
-* [Contributing](./CONTRIBUTING.md)
+# What is the Cluster API?
+The Cluster API is a Kubernetes project to bring declarative, Kubernetes-style APIs to cluster creation,
+configuration, and management. It provides optional, additive functionality on top of core Kubernetes to
+manage the lifecycle of a Kubernetes cluster.
 
-**Using Cluster API v1alpha2?** See the [legacy
-documentation](https://release-0-2.cluster-api.sigs.k8s.io).
-
-
-## Why build Cluster API?
-
-Kubernetes is a complex system that relies on several components being configured correctly to have a working cluster. Recognizing this as a potential stumbling block for users, the community focused on simplifying the bootstrapping process. Today, over [100 Kubernetes distributions and installers](https://www.cncf.io/certification/software-conformance/) have been created, each with different default configurations for clusters and supported infrastructure providers. SIG Cluster Lifecycle saw a need for a single tool to address a set of common overlapping installation concerns and started kubeadm.
-
-[Kubeadm](https://kubernetes.io/docs/reference/setup-tools/kubeadm/kubeadm/) was designed as a focused tool for bootstrapping a best-practices Kubernetes cluster. The core tenet behind the kubeadm project was to create a tool that other installers can leverage and ultimately alleviate the amount of configuration that an individual installer needed to maintain. Since it began, kubeadm has become the underlying bootstrapping tool for several other applications, including Kubespray, Minikube, kind, etc.
-
-However, while kubeadm and other bootstrap providers reduce installation complexity, they don't address how to manage a cluster day-to-day or a Kubernetes environment long term. You are still faced with several questions when setting up a production environment, including
-
-* How can I consistently provision machines, load balancers, VPC, etc., across multiple infrastructure providers and locations?
-* How can I automate cluster lifecycle management, including things like upgrades and cluster deletion?
-* How can I scale these processes to manage any number of clusters?
-
-SIG Cluster Lifecycle began the Cluster API project as a way to address these gaps by building declarative, Kubernetes-style APIs, that automate cluster creation, configuration, and management. Using this model, Cluster API can also be extended to support any infrastructure provider (AWS, Azure, vSphere, etc.) or bootstrap provider (kubeadm is default) you need. See the growing list of [available providers](./reference/providers.md).
 
 {{#include ../../scope-and-objectives.md:Goals}}
 
diff --git a/go.mod b/go.mod
index 3f8a47830..90342e39e 100644
--- a/go.mod
+++ b/go.mod
@@ -38,7 +38,7 @@ require (
 	k8s.io/component-base v0.17.8
 	k8s.io/klog v1.0.0
 	k8s.io/utils v0.0.0-20200619165400-6e3d28b6ed19
-	sigs.k8s.io/controller-runtime v0.5.9
+	sigs.k8s.io/controller-runtime v0.5.8
 	sigs.k8s.io/kind v0.7.1-0.20200303021537-981bd80d3802
 	sigs.k8s.io/yaml v1.2.0
 )
diff --git a/go.sum b/go.sum
index 01cf45df9..fad368654 100644
--- a/go.sum
+++ b/go.sum
@@ -646,8 +646,8 @@ modernc.org/golex v1.0.0/go.mod h1:b/QX9oBD/LhixY6NDh+IdGv17hgB+51fET1i2kPSmvk=
 modernc.org/mathutil v1.0.0/go.mod h1:wU0vUrJsVWBZ4P6e7xtFJEhFSNsfRLJ8H458uRjg03k=
 modernc.org/strutil v1.0.0/go.mod h1:lstksw84oURvj9y3tn8lGvRxyRC1S2+g5uuIzNfIOBs=
 modernc.org/xc v1.0.0/go.mod h1:mRNCo0bvLjGhHO9WsyuKVU4q0ceiDDDoEeWDJHrNx8I=
-sigs.k8s.io/controller-runtime v0.5.9 h1:ccbiT+HK8Eyw252Nv8wZi84iWVOQVDha6NU2w2/yML4=
-sigs.k8s.io/controller-runtime v0.5.9/go.mod h1:UI/unU7Q+mo/rWBrND0NAaVNj/Xjh/+aqSv/M3njpmo=
+sigs.k8s.io/controller-runtime v0.5.8 h1:+pp4plYh2rpjuVo6HBJ1pVgN3cvAfQHfkKK27rLdxxI=
+sigs.k8s.io/controller-runtime v0.5.8/go.mod h1:UI/unU7Q+mo/rWBrND0NAaVNj/Xjh/+aqSv/M3njpmo=
 sigs.k8s.io/kind v0.7.1-0.20200303021537-981bd80d3802 h1:L6/8hETA7jvdx3xBcbDifrIN2xaYHE7tA58n+Kdp2Zw=
 sigs.k8s.io/kind v0.7.1-0.20200303021537-981bd80d3802/go.mod h1:HIZ3PWUezpklcjkqpFbnYOqaqsAE1JeCTEwkgvPLXjk=
 sigs.k8s.io/structured-merge-diff v0.0.0-20190525122527-15d366b2352e/go.mod h1:wWxsB5ozmmv/SG7nM11ayaAW51xMvak/t1r0CSlcokI=
diff --git a/util/conditions/patch.go b/util/conditions/patch.go
index 4e2bd6f00..9d4d91771 100644
--- a/util/conditions/patch.go
+++ b/util/conditions/patch.go
@@ -29,8 +29,8 @@ type Patch []PatchOperation
 
 // PatchOperation define an operation that changes a single condition.
 type PatchOperation struct {
-	Before *clusterv1.Condition
 	After  *clusterv1.Condition
+	Before *clusterv1.Condition
 	Op     PatchOperationType
 }
 
@@ -129,7 +129,7 @@ func (p Patch) Apply(latest Setter, options ...ApplyOption) error {
 			if latestCondition := Get(latest, conditionPatch.After.Type); latestCondition != nil {
 				// If latest and after agree on the change, then it is a conflict.
 				if !hasSameState(latestCondition, conditionPatch.After) {
-					return errors.Errorf("error patching conditions: The condition %q was modified by a different process and this caused a merge/AddCondition conflict: %v", conditionPatch.After.Type, cmp.Diff(latestCondition, conditionPatch.After))
+					return errors.Errorf("error patching conditions: The condition %q was modified by a different process and this caused a merge/AddCondition conflict: %v", conditionPatch.After.Type, cmp.Diff(conditionPatch.Before, conditionPatch.After))
 				}
 				// otherwise, the latest is already as intended.
 				// NOTE: We are preserving LastTransitionTime from the latest in order to avoid altering the existing value.
@@ -149,14 +149,14 @@ func (p Patch) Apply(latest Setter, options ...ApplyOption) error {
 
 			// If the condition does not exist anymore on the latest, this is a conflict.
 			if latestCondition == nil {
-				return errors.Errorf("error patching conditions: The condition %q was deleted by a different process and this caused a merge/ChangeCondition conflict", conditionPatch.After.Type)
+				return errors.Errorf("error patching conditions: The condition %q was deleted by a different process and this caused a merge/ChangeCondition conflict: %v", conditionPatch.After.Type, cmp.Diff(conditionPatch.Before, conditionPatch.After))
 			}
 
 			// If the condition on the latest is different from the base condition, check if
 			// the after state corresponds to the desired value. If not this is a conflict (unless we should ignore conflicts for this condition type).
 			if !reflect.DeepEqual(latestCondition, conditionPatch.Before) {
 				if !hasSameState(latestCondition, conditionPatch.After) {
-					return errors.Errorf("error patching conditions: The condition %q was modified by a different process and this caused a merge/ChangeCondition conflict: %v", conditionPatch.After.Type, cmp.Diff(latestCondition, conditionPatch.After))
+					return errors.Errorf("error patching conditions: The condition %q was modified by a different process and this caused a merge/ChangeCondition conflict: %v", conditionPatch.After.Type, cmp.Diff(conditionPatch.Before, conditionPatch.After))
 				}
 				// Otherwise the latest is already as intended.
 				// NOTE: We are preserving LastTransitionTime from the latest in order to avoid altering the existing value.
@@ -176,7 +176,7 @@ func (p Patch) Apply(latest Setter, options ...ApplyOption) error {
 			// if so then this is a conflict.
 			if latestCondition := Get(latest, conditionPatch.Before.Type); latestCondition != nil {
 				if !hasSameState(latestCondition, conditionPatch.Before) {
-					return errors.Errorf("error patching conditions: The condition %q was modified by a different process and this caused a merge/RemoveCondition conflict: %v", conditionPatch.Before.Type, cmp.Diff(latestCondition, conditionPatch.Before))
+					return errors.Errorf("error patching conditions: The condition %q was modified by a different process and this caused a merge/RemoveCondition conflict: %v", conditionPatch.Before.Type, cmp.Diff(conditionPatch.Before, conditionPatch.After))
 				}
 			}
 			// Otherwise the latest and after agreed on the delete operation, so there's nothing to change.


Test patch:
diff --git a/test/infrastructure/docker/go.mod b/test/infrastructure/docker/go.mod
index da2d30008..6629755a8 100644
--- a/test/infrastructure/docker/go.mod
+++ b/test/infrastructure/docker/go.mod
@@ -11,7 +11,7 @@ require (
 	k8s.io/client-go v0.17.8
 	k8s.io/klog v1.0.0
 	sigs.k8s.io/cluster-api v0.3.3
-	sigs.k8s.io/controller-runtime v0.5.9
+	sigs.k8s.io/controller-runtime v0.5.8
 	sigs.k8s.io/kind v0.7.1-0.20200303021537-981bd80d3802
 	sigs.k8s.io/yaml v1.2.0
 )
diff --git a/test/infrastructure/docker/go.sum b/test/infrastructure/docker/go.sum
index f22396227..0302f10b8 100644
--- a/test/infrastructure/docker/go.sum
+++ b/test/infrastructure/docker/go.sum
@@ -606,8 +606,8 @@ modernc.org/golex v1.0.0/go.mod h1:b/QX9oBD/LhixY6NDh+IdGv17hgB+51fET1i2kPSmvk=
 modernc.org/mathutil v1.0.0/go.mod h1:wU0vUrJsVWBZ4P6e7xtFJEhFSNsfRLJ8H458uRjg03k=
 modernc.org/strutil v1.0.0/go.mod h1:lstksw84oURvj9y3tn8lGvRxyRC1S2+g5uuIzNfIOBs=
 modernc.org/xc v1.0.0/go.mod h1:mRNCo0bvLjGhHO9WsyuKVU4q0ceiDDDoEeWDJHrNx8I=
-sigs.k8s.io/controller-runtime v0.5.9 h1:ccbiT+HK8Eyw252Nv8wZi84iWVOQVDha6NU2w2/yML4=
-sigs.k8s.io/controller-runtime v0.5.9/go.mod h1:UI/unU7Q+mo/rWBrND0NAaVNj/Xjh/+aqSv/M3njpmo=
+sigs.k8s.io/controller-runtime v0.5.8 h1:+pp4plYh2rpjuVo6HBJ1pVgN3cvAfQHfkKK27rLdxxI=
+sigs.k8s.io/controller-runtime v0.5.8/go.mod h1:UI/unU7Q+mo/rWBrND0NAaVNj/Xjh/+aqSv/M3njpmo=
 sigs.k8s.io/kind v0.7.1-0.20200303021537-981bd80d3802 h1:L6/8hETA7jvdx3xBcbDifrIN2xaYHE7tA58n+Kdp2Zw=
 sigs.k8s.io/kind v0.7.1-0.20200303021537-981bd80d3802/go.mod h1:HIZ3PWUezpklcjkqpFbnYOqaqsAE1JeCTEwkgvPLXjk=
 sigs.k8s.io/structured-merge-diff v0.0.0-20190525122527-15d366b2352e/go.mod h1:wWxsB5ozmmv/SG7nM11ayaAW51xMvak/t1r0CSlcokI=