Skip to content
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.

Sign up for GitHub

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

Better management of status and conditions for externalIP #169

Open
wants to merge 1 commit into
base: release/0.13.0
Choose a base branch
from
Open

Better management of status and conditions for externalIP #169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions api/v1alpha1/externalip_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,24 @@ type ExternalIPState string

// All defined ExternalIPStates
const (
ExternalIPStateNone ExternalIPState = ""
ExternalIPStateReserved ExternalIPState = "Reserved"
ExternalIPStatePending ExternalIPState = "Pending"
ExternalIPStateAssociated ExternalIPState = "Associated"
)

// The list of condition types.
const (
ExternalIPConditionTypeIPCreated = "IPCreated"
ExternalIPConditionTypeNetworkInterfaceAssociated = "NetworkInterfaceAssociated"
)

// The list of condition reasons.
const (
ExternalIPConditionReasonProviderError = "ProviderError"
ExternalIPConditionReasonIPCreated = "IPCreated"
ExternalIPConditionReasonNetworkInterfaceAssociated = "NetworkInterfaceAssociated"
)

// ExternalIPStatus defines the observed state of ExternalIP
type ExternalIPStatus struct {
// The current state of the ExternalIP
Expand All @@ -60,6 +73,10 @@ type ExternalIPStatus struct {

// The instance identifier
InstanceID *string `json:"instanceID,omitempty"`

// Current conditions of the ExternalIP.
// +kubebuilder:validation:Optional
Conditions []metav1.Condition `json:"conditions,omitempty"`
}

// +kubebuilder:object:root=true
Expand All @@ -78,11 +95,6 @@ type ExternalIP struct {
Status ExternalIPStatus `json:"status,omitempty"`
}

// IsReserved returns if externalIP is reserved
func (e *ExternalIP) IsReserved() bool {
return e.Status.State == ExternalIPStateReserved
}

// IsAssociated returns if externalIP is associated
func (e *ExternalIP) IsAssociated() bool {
return e.Status.State == ExternalIPStateAssociated
Expand Down
7 changes: 7 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

57 changes: 57 additions & 0 deletions config/crd/bases/kubestatic.quortex.io_externalips.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,63 @@ spec:
addressID:
description: The address identifier
type: string
conditions:
description: Current conditions of the ExternalIP.
items:
description: Condition contains details for one aspect of the current
state of this API Resource.
properties:
lastTransitionTime:
description: |-
lastTransitionTime is the last time the condition transitioned from one status to another.
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
format: date-time
type: string
message:
description: |-
message is a human readable message indicating details about the transition.
This may be an empty string.
maxLength: 32768
type: string
observedGeneration:
description: |-
observedGeneration represents the .metadata.generation that the condition was set based upon.
For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date
with respect to the current state of the instance.
format: int64
minimum: 0
type: integer
reason:
description: |-
reason contains a programmatic identifier indicating the reason for the condition's last transition.
Producers of specific condition types may define expected values and meanings for this field,
and whether the values are considered a guaranteed API.
The value should be a CamelCase string.
This field may not be empty.
maxLength: 1024
minLength: 1
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
type: string
status:
description: status of the condition, one of True, False, Unknown.
enum:
- "True"
- "False"
- Unknown
type: string
type:
description: type of condition in CamelCase or in foo.example.com/CamelCase.
maxLength: 316
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
type: string
required:
- lastTransitionTime
- message
- reason
- status
- type
type: object
type: array
instanceID:
description: The instance identifier
type: string
Expand Down
56 changes: 56 additions & 0 deletions helm/kubestatic/crds/crds.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,62 @@ spec:
addressID:
description: The address identifier
type: string
conditions:
description: Current conditions of the ExternalIP.
items:
description: Condition contains details for one aspect of the current state of this API Resource.
properties:
lastTransitionTime:
description: |-
lastTransitionTime is the last time the condition transitioned from one status to another.
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
format: date-time
type: string
message:
description: |-
message is a human readable message indicating details about the transition.
This may be an empty string.
maxLength: 32768
type: string
observedGeneration:
description: |-
observedGeneration represents the .metadata.generation that the condition was set based upon.
For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date
with respect to the current state of the instance.
format: int64
minimum: 0
type: integer
reason:
description: |-
reason contains a programmatic identifier indicating the reason for the condition's last transition.
Producers of specific condition types may define expected values and meanings for this field,
and whether the values are considered a guaranteed API.
The value should be a CamelCase string.
This field may not be empty.
maxLength: 1024
minLength: 1
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
type: string
status:
description: status of the condition, one of True, False, Unknown.
enum:
- "True"
- "False"
- Unknown
type: string
type:
description: type of condition in CamelCase or in foo.example.com/CamelCase.
maxLength: 316
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
type: string
required:
- lastTransitionTime
- message
- reason
- status
- type
type: object
type: array
instanceID:
description: The instance identifier
type: string
Expand Down
57 changes: 42 additions & 15 deletions internal/controller/externalip_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ package controller
import (
"context"
"encoding/json"
"errors"
"fmt"
"reflect"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/equality"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/strategicpatch"
Expand Down Expand Up @@ -63,7 +64,7 @@ func (r *ExternalIPReconciler) Reconcile(ctx context.Context, req ctrl.Request)

externalIP := &v1alpha1.ExternalIP{}
if err := r.Get(ctx, req.NamespacedName, externalIP); err != nil {
if errors.IsNotFound(err) {
if apierrors.IsNotFound(err) {
// Request object not found, could have been deleted after reconcile request.
// Return and don't requeue
log.Info("ExternalIP resource not found. Ignoring since object must be deleted")
Expand Down Expand Up @@ -95,7 +96,7 @@ func (r *ExternalIPReconciler) Reconcile(ctx context.Context, req ctrl.Request)
var instanceID string
if externalIP.Spec.NodeName != "" {
if err := r.Get(ctx, types.NamespacedName{Name: externalIP.Spec.NodeName}, &node); err != nil {
if errors.IsNotFound(err) {
if apierrors.IsNotFound(err) {
// Invalid nodeName, remove ExternalIP nodeName attribute.
externalIP.Spec.NodeName = ""
if err != r.Update(ctx, externalIP) {
Expand All @@ -118,6 +119,10 @@ func (r *ExternalIPReconciler) Reconcile(ctx context.Context, req ctrl.Request)
if externalIP.ObjectMeta.DeletionTimestamp.IsZero() {
status, err = r.Provider.ReconcileExternalIP(ctx, log, instanceID, externalIP)
if err != nil {
if patchErr := patchExternalIPStatus(ctx, r, externalIP, status); patchErr != nil {
log.Error(errors.Join(patchErr, err), "Failed to patch ExternalIP status during error handling")
return ctrl.Result{}, fmt.Errorf("failed to patch ExternalIP status during error handling: %w", errors.Join(patchErr, err))
}
log.Error(err, "Failed to reconcile ExternalIP")
return ctrl.Result{}, err
}
Expand Down Expand Up @@ -168,17 +173,9 @@ func (r *ExternalIPReconciler) Reconcile(ctx context.Context, req ctrl.Request)
return ctrl.Result{}, nil
}

// Copy the existing ExternalIP to avoid mutating the original
existingExternalIP := externalIP.DeepCopy()

// Patch the Pool status if it differs from the desired status
externalIP.Status = status
if !reflect.DeepEqual(externalIP.Status, existingExternalIP.Status) {
err := r.Status().Patch(ctx, externalIP, client.MergeFrom(existingExternalIP))
if err != nil {
log.Error(err, "Failed to patch ExternalIP status")
return ctrl.Result{}, fmt.Errorf("failed to patch ExternalIP status: %w", err)
}
if err := patchExternalIPStatus(ctx, r, externalIP, status); err != nil {
log.Error(err, "Failed to patch ExternalIP status")
return ctrl.Result{}, fmt.Errorf("failed to patch ExternalIP status: %w", err)
}

log.Info("ExternalIP successfully reconciled")
Expand Down Expand Up @@ -215,3 +212,33 @@ func (r *ExternalIPReconciler) SetupWithManager(mgr ctrl.Manager) error {
).
Complete(r)
}

// patchExternalIPStatus updates the status of a ExternalIP resource if there are any changes.
// It patches the status with the new status provided and updates the LastTransitionTime if there are differences.
//
// Parameters:
//
// ctx - The context for the request.
// r - The ExternalIPReconciler responsible for reconciling the ExternalIP resource.
// externalIP - The ExternalIP resource to be updated.
// newStatus - The new status to be applied to the ExternalIP resource.
//
// Returns:
//
// error - An error if the patch operation fails, otherwise nil.
func patchExternalIPStatus(
ctx context.Context,
r *ExternalIPReconciler,
externalIP *v1alpha1.ExternalIP,
newStatus v1alpha1.ExternalIPStatus,
) error {
existingEIP := externalIP.DeepCopy()
externalIP.Status = newStatus

if !equality.Semantic.DeepEqual(externalIP.Status, existingEIP.Status) {
if err := r.Status().Patch(ctx, externalIP, client.MergeFrom(existingEIP)); err != nil {
return err
}
}
return nil
}
3 changes: 3 additions & 0 deletions internal/provider/aws/converter/error_decoder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ func DecodeCommonError(msg string, err error) error {
"MalformedQueryString",
"ValidationError":
return &provider.Error{Code: provider.BadRequestError, Msg: msg}
case
"AddressLimitExceeded":
return &provider.Error{Code: provider.AddressLimitExceededError, Msg: msg}
case
"RulesPerSecurityGroupLimitExceeded",
"Throttling":
Expand Down
Loading
Loading