Merge pull request #32117 from nebril/petset-count

Automatic merge from submit-queue

PetSet returns valid replica count in status

**What this PR does / why we need it**: It prevents the PetSet replica count to be invalid regardless of pods not being created due to 

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #31965

**Special notes for your reviewer**:

**Release note**:
```release-note
```

pkg/controller/petset/pet.go

@@ -81,6 +81,15 @@ type petSyncer struct {
 	blockingPet *pcb
 }
 
+// errUnhealthyPet is returned when a we either know for sure a pet is unhealthy,
+// or don't know its state but assume it is unhealthy. It's used as a signal to the caller for further operations like updating status.replicas.
+// This is not a fatal error.
+type errUnhealthyPet string
+
+func (e errUnhealthyPet) Error() string {
+	return string(e)
+}
+
 // Sync syncs the given pet.
 func (p *petSyncer) Sync(pet *pcb) error {
 	if pet == nil {
@@ -103,8 +112,9 @@ func (p *petSyncer) Sync(pet *pcb) error {
 		return p.Update(realPet, pet)
 	}
 	if p.blockingPet != nil {
-		glog.Infof("Create of %v in PetSet %v blocked by unhealthy pet %v", pet.pod.Name, pet.parent.Name, p.blockingPet.pod.Name)
-		return nil
+		message := errUnhealthyPet(fmt.Sprintf("Create of %v in PetSet %v blocked by unhealthy pet %v", pet.pod.Name, pet.parent.Name, p.blockingPet.pod.Name))
+		glog.Info(message)
+		return message
 	}
 	// This is counted as a create, even if it fails. We can't skip indices
 	// because some pets might allocate a special role to earlier indices.

pkg/controller/petset/pet_set.go

@@ -348,7 +348,14 @@ func (psc *PetSetController) syncPetSet(ps *apps.PetSet, pets []*api.Pod) (int,
 		case deletePet:
 			err = petManager.Delete(pet)
 		}
-		if err != nil {
+		switch err.(type) {
+		case errUnhealthyPet:
+			// We are not passing this error up, but we don't increment numPets if we encounter it,
+			// since numPets directly translates to petset.status.replicas
+			continue
+		case nil:
+			continue
+		default:
 			it.errs = append(it.errs, err)
 		}
 	}

pkg/controller/petset/pet_set_test.go

@@ -267,3 +267,13 @@ func TestPetSetBlockingPetIsCleared(t *testing.T) {
 		t.Errorf("Unexpected blocking pet, err %v: %+v", err, p)
 	}
 }
+
+// TODO(mkwiek): test if the petset.Status.Replicas is actually correct
+func TestPetSetReplicaCount(t *testing.T) {
+	psc, fc := newFakePetSetController()
+	ps := newPetSet(3)
+	i, _ := psc.syncPetSet(ps, fc.getPodList())
+	if i != len(fc.getPodList()) {
+		t.Errorf("syncPetSet should return actual amount of pods")
+	}
+}