-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve logic for setting LastTransitionTime #16
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,59 +17,65 @@ limitations under the License. | |
package conditions | ||
|
||
import ( | ||
cmutil "github.com/cert-manager/cert-manager/pkg/api/util" | ||
cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" | ||
cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
"k8s.io/apimachinery/pkg/runtime" | ||
"k8s.io/utils/clock" | ||
) | ||
|
||
// private struct; only used to implement the GenericIssuer interface | ||
// for use with the cmutil.SetIssuerCondition function | ||
type genericIssuer struct { | ||
metav1.TypeMeta `json:",inline"` | ||
metav1.ObjectMeta `json:"metadata,omitempty"` | ||
|
||
Status cmapi.IssuerStatus `json:"status"` | ||
} | ||
|
||
func (g *genericIssuer) DeepCopyObject() runtime.Object { | ||
panic("[HACK]: this function should not get called") | ||
} | ||
|
||
func (g *genericIssuer) GetSpec() *cmapi.IssuerSpec { | ||
panic("[HACK]: this function should not get called") | ||
} | ||
|
||
func (g *genericIssuer) GetObjectMeta() *metav1.ObjectMeta { | ||
return &g.ObjectMeta | ||
} | ||
|
||
func (g *genericIssuer) GetStatus() *cmapi.IssuerStatus { | ||
return &g.Status | ||
} | ||
|
||
// Update the status with the provided condition details & return | ||
// the added condition. | ||
// NOTE: this code is just a workaround for cmutil only accepting a GenericIssuer interface | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of calling the cert-manager code, we now just host this logic in this repo. This makes it much clearer what is going on & allowed me to customize the logic for our needs (SSA). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely makes sense for his repo's issuer type to have these kinds of functions be more specific! |
||
func SetIssuerStatusCondition( | ||
conditions *[]cmapi.IssuerCondition, | ||
clock clock.PassiveClock, | ||
existingConditions []cmapi.IssuerCondition, | ||
patchConditions *[]cmapi.IssuerCondition, | ||
observedGeneration int64, | ||
conditionType cmapi.IssuerConditionType, | ||
status cmmeta.ConditionStatus, | ||
reason, message string, | ||
) *cmapi.IssuerCondition { | ||
gi := genericIssuer{ | ||
Status: cmapi.IssuerStatus{ | ||
Conditions: *conditions, | ||
}, | ||
) (*cmapi.IssuerCondition, *metav1.Time) { | ||
newCondition := cmapi.IssuerCondition{ | ||
Type: conditionType, | ||
Status: status, | ||
Reason: reason, | ||
Message: message, | ||
ObservedGeneration: observedGeneration, | ||
} | ||
|
||
cmutil.SetIssuerCondition(&gi, observedGeneration, conditionType, status, reason, message) | ||
nowTime := metav1.NewTime(clock.Now()) | ||
newCondition.LastTransitionTime = &nowTime | ||
|
||
// Reset the LastTransitionTime if the status hasn't changed | ||
for _, cond := range existingConditions { | ||
if cond.Type != conditionType { | ||
continue | ||
} | ||
|
||
// If this update doesn't contain a state transition, we don't update | ||
// the conditions LastTransitionTime to Now() | ||
if cond.Status == status { | ||
newCondition.LastTransitionTime = cond.LastTransitionTime | ||
} | ||
} | ||
|
||
// Search through existing conditions | ||
for idx, cond := range *patchConditions { | ||
// Skip unrelated conditions | ||
if cond.Type != conditionType { | ||
continue | ||
} | ||
|
||
// Overwrite the existing condition | ||
(*patchConditions)[idx] = newCondition | ||
|
||
return &newCondition, &nowTime | ||
} | ||
|
||
*conditions = gi.Status.Conditions | ||
// If we've not found an existing condition of this type, we simply insert | ||
// the new condition into the slice. | ||
*patchConditions = append(*patchConditions, newCondition) | ||
|
||
return GetIssuerStatusCondition(*conditions, conditionType) | ||
return &newCondition, &nowTime | ||
} | ||
|
||
func GetIssuerStatusCondition( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment: It's a shame to have this code here - functions which act on cert-manager resources such as CertificateRequest seem like a natural fit for the cert-manager codebase. Better to have one source of truth for "how do I update a CR status condition".
Don't think it's a dealbreaker - but I wonder if we could add something to the cert-manager codebase instead? Doesn't really matter though