Skip to content

Commit 749d72a

Browse files
committed
Reset network status when CSR is deleted
The signer controller is reflecting CSR approval status into network operator status during signing process, but when CSR is removed it's not getting network status back into original state because CSR is no longer available. When CSR signing reattempt happens, signer controller is not updating existing CertificateFailed condition type, instead it tries to add another CertificateFailed condition and leads to Duplicate value: "Failed" error, so fixing it just by updating existing CertificateFailed condition. Signed-off-by: Periyasamy Palanisamy <[email protected]>
1 parent 0508612 commit 749d72a

File tree

2 files changed

+126
-1
lines changed

2 files changed

+126
-1
lines changed

pkg/controller/signer/signer-controller.go

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"log"
7+
"time"
78

89
cnoclient "github.com/openshift/cluster-network-operator/pkg/client"
910
"github.com/openshift/cluster-network-operator/pkg/controller/statusmanager"
@@ -88,6 +89,8 @@ func (r *ReconcileCSR) Reconcile(ctx context.Context, request reconcile.Request)
8889
if err != nil {
8990
if apierrors.IsNotFound(err) {
9091
// Request object not found, could have been deleted after reconcile request.
92+
// restore network status when CSR is deleted.
93+
r.status.SetNotDegraded(statusmanager.CertificateSigner)
9194
// Return and don't requeue as the CSR has been deleted.
9295
return reconcile.Result{}, nil
9396
}
@@ -251,7 +254,7 @@ func signerFailure(r *ReconcileCSR, csr *csrv1.CertificateSigningRequest, reason
251254

252255
// Update the status conditions on the CSR object
253256
func updateCSRStatusConditions(r *ReconcileCSR, csr *csrv1.CertificateSigningRequest, reason string, message string) {
254-
csr.Status.Conditions = append(csr.Status.Conditions, csrv1.CertificateSigningRequestCondition{
257+
setCertificateSigningRequestCondition(&csr.Status.Conditions, csrv1.CertificateSigningRequestCondition{
255258
Type: csrv1.CertificateFailed,
256259
Status: "True",
257260
Reason: reason,
@@ -264,3 +267,26 @@ func updateCSRStatusConditions(r *ReconcileCSR, csr *csrv1.CertificateSigningReq
264267
fmt.Sprintf("Unable to update csr: %v", err))
265268
}
266269
}
270+
271+
func setCertificateSigningRequestCondition(conditions *[]csrv1.CertificateSigningRequestCondition, newCondition csrv1.CertificateSigningRequestCondition) {
272+
if conditions == nil {
273+
conditions = &[]csrv1.CertificateSigningRequestCondition{}
274+
}
275+
var existingCondition *csrv1.CertificateSigningRequestCondition
276+
for i := range *conditions {
277+
if (*conditions)[i].Type == newCondition.Type {
278+
existingCondition = &(*conditions)[i]
279+
}
280+
}
281+
if existingCondition == nil {
282+
newCondition.LastTransitionTime = metav1.NewTime(time.Now())
283+
*conditions = append(*conditions, newCondition)
284+
return
285+
}
286+
if existingCondition.Status != newCondition.Status {
287+
existingCondition.Status = newCondition.Status
288+
existingCondition.LastTransitionTime = metav1.NewTime(time.Now())
289+
}
290+
existingCondition.Reason = newCondition.Reason
291+
existingCondition.Message = newCondition.Message
292+
}

pkg/controller/signer/signer_test.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,105 @@ func TestSigner_reconciler_withInvalidUserName(t *testing.T) {
160160
g.Expect(csrConditions[0].Type).To(Equal(certificatev1.CertificateFailed))
161161
}
162162

163+
func TestSigner_DegradedCondition(t *testing.T) {
164+
g := NewGomegaWithT(t)
165+
client := fake.NewFakeClient()
166+
status := statusmanager.New(client, coName, names.StandAloneClusterName)
167+
signer := ReconcileCSR{client: client, status: status}
168+
169+
co := &configv1.ClusterOperator{ObjectMeta: metav1.ObjectMeta{Name: coName}}
170+
setCO(t, client, co)
171+
no := &operv1.Network{ObjectMeta: metav1.ObjectMeta{Name: names.OPERATOR_CONFIG}}
172+
setOC(t, client, no)
173+
174+
csr, err := generateCSR()
175+
g.Expect(err).NotTo(HaveOccurred())
176+
csrObj := &certificatev1.CertificateSigningRequest{}
177+
csrObj.Name = csrName
178+
csrObj.Spec.Request = []byte(csr)
179+
csrObj.Spec.SignerName = signerName
180+
csrObj.Spec.Usages = []certificatev1.KeyUsage{"ipsec tunnel"}
181+
csrObj.Spec.Username = fmt.Sprintf("system:ovn-node:%s", nodeName)
182+
csrObj.Status.Conditions = append(csrObj.Status.Conditions, certificatev1.CertificateSigningRequestCondition{
183+
Type: certificatev1.CertificateApproved,
184+
Status: "True",
185+
Reason: "AutoApproved",
186+
Message: "Automatically approved by " + signerName})
187+
188+
err = client.Default().CRClient().Create(context.TODO(), csrObj)
189+
g.Expect(err).NotTo(HaveOccurred())
190+
_, err = client.Default().Kubernetes().CertificatesV1().CertificateSigningRequests().Create(context.TODO(), csrObj, metav1.CreateOptions{})
191+
g.Expect(err).NotTo(HaveOccurred())
192+
193+
node := &corev1.Node{}
194+
node.Name = nodeName
195+
_, err = client.Default().Kubernetes().CoreV1().Nodes().Create(context.TODO(), node, metav1.CreateOptions{})
196+
g.Expect(err).NotTo(HaveOccurred())
197+
198+
randomByteArray := []byte("blahblahblah")
199+
caSecret := &corev1.Secret{}
200+
caSecret.Name = "signer-ca"
201+
caSecret.Namespace = "openshift-ovn-kubernetes"
202+
caSecret.Data = make(map[string][]byte)
203+
caSecret.Data["tls.crt"] = randomByteArray
204+
caSecret.Data["tls.key"] = randomByteArray
205+
err = client.Default().CRClient().Create(context.TODO(), caSecret)
206+
g.Expect(err).NotTo(HaveOccurred())
207+
208+
for range 3 {
209+
_, err = signer.Reconcile(context.TODO(),
210+
reconcile.Request{NamespacedName: types.NamespacedName{Name: csrName}})
211+
g.Expect(err).NotTo(HaveOccurred())
212+
213+
err = client.Default().CRClient().Get(context.TODO(), types.NamespacedName{Name: csrName}, csrObj)
214+
g.Expect(err).NotTo(HaveOccurred())
215+
g.Expect(csrObj.Status.Certificate).Should(BeEmpty())
216+
217+
co, _, err = getStatuses(client, "testing")
218+
if err != nil {
219+
t.Fatalf("error getting network.operator: %v", err)
220+
}
221+
g.Expect(err).NotTo(HaveOccurred())
222+
g.Expect(conditionsInclude(co.Status.Conditions, []configv1.ClusterOperatorStatusCondition{
223+
{
224+
Type: configv1.OperatorDegraded,
225+
Status: configv1.ConditionTrue,
226+
},
227+
})).To(BeTrue())
228+
g.Expect(conditionsInclude(co.Status.Conditions, []configv1.ClusterOperatorStatusCondition{
229+
{
230+
Type: configv1.OperatorUpgradeable,
231+
Status: configv1.ConditionTrue,
232+
},
233+
})).To(BeTrue())
234+
}
235+
236+
err = client.Default().CRClient().Delete(context.TODO(), csrObj)
237+
g.Expect(err).NotTo(HaveOccurred())
238+
_, err = signer.Reconcile(context.TODO(),
239+
reconcile.Request{NamespacedName: types.NamespacedName{Name: csrName}})
240+
g.Expect(err).NotTo(HaveOccurred())
241+
242+
co, _, err = getStatuses(client, "testing")
243+
if err != nil {
244+
t.Fatalf("error getting network.operator: %v", err)
245+
}
246+
g.Expect(err).NotTo(HaveOccurred())
247+
g.Expect(conditionsInclude(co.Status.Conditions, []configv1.ClusterOperatorStatusCondition{
248+
{
249+
Type: configv1.OperatorDegraded,
250+
Status: configv1.ConditionFalse,
251+
},
252+
})).To(BeTrue())
253+
g.Expect(conditionsInclude(co.Status.Conditions, []configv1.ClusterOperatorStatusCondition{
254+
{
255+
Type: configv1.OperatorUpgradeable,
256+
Status: configv1.ConditionTrue,
257+
},
258+
})).To(BeTrue())
259+
260+
}
261+
163262
func generateCSR() (string, error) {
164263
// Create private key.
165264
csrKey, err := rsa.GenerateKey(rand.Reader, 2048)

0 commit comments

Comments
 (0)