Skip to content

Commit

Permalink
update according to code review notes
Browse files Browse the repository at this point in the history
Signed-off-by: Artem Bortnikov <artem.bortnikov@telekom.com>
  • Loading branch information
aobort committed Jun 6, 2024
1 parent a8c04d2 commit 0782ef6
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 18 deletions.
13 changes: 13 additions & 0 deletions internal/controller/factory/builders.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
)

// contextWithGVK returns a new context with the GroupVersionKind (GVK) information
// of the given resource added to the context values. It uses the provided scheme
// to determine the GVK.
// If the resource is nil, it returns an error with the message "resource cannot be nil".
// If there is an error while obtaining the GVK, it returns an error with the message
// "failed to get GVK" followed by the detailed error message.
// The context value is updated with the following key-value pairs:
// - "group": GVK's GroupVersion string
// - "kind": GVK's Kind
// - "name": Resource's name
func contextWithGVK(ctx context.Context, resource client.Object, scheme *runtime.Scheme) (context.Context, error) {
if resource == nil {
return nil, fmt.Errorf("resource cannot be nil")
Expand Down Expand Up @@ -63,6 +73,9 @@ func reconcileOwnedResource(ctx context.Context, c client.Client, resource clien
}

func deleteOwnedResource(ctx context.Context, c client.Client, resource client.Object) error {
if resource == nil {
return fmt.Errorf("resource cannot be nil")
}
log.Debug(ctx, "deleting owned resource")
return client.IgnoreNotFound(c.Delete(ctx, resource))
}
42 changes: 24 additions & 18 deletions internal/controller/factory/pdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,37 +35,43 @@ func CreateOrUpdatePdb(
cluster *etcdaenixiov1alpha1.EtcdCluster,
rclient client.Client,
) error {
var err error

if cluster.Spec.PodDisruptionBudgetTemplate == nil {
ctx = log.WithValues(ctx, "group", "policy/v1", "kind", "PodDisruptionBudget", "name", cluster.Name)
return deleteOwnedResource(ctx, rclient, &v1.PodDisruptionBudget{
ObjectMeta: metav1.ObjectMeta{
Namespace: cluster.Namespace,
Name: cluster.Name,
},
})
}

pdb := &v1.PodDisruptionBudget{
ObjectMeta: metav1.ObjectMeta{
Namespace: cluster.Namespace,
Name: cluster.Name,
},
}
ctx, err := contextWithGVK(ctx, pdb, rclient.Scheme())
if err != nil {
return err
}

if cluster.Spec.PodDisruptionBudgetTemplate == nil {
return deleteOwnedResource(ctx, rclient, pdb)
}

pdb.Spec = v1.PodDisruptionBudgetSpec{
MinAvailable: cluster.Spec.PodDisruptionBudgetTemplate.Spec.MinAvailable,
MaxUnavailable: cluster.Spec.PodDisruptionBudgetTemplate.Spec.MaxUnavailable,
Selector: &metav1.LabelSelector{
MatchLabels: NewLabelsBuilder().WithName().WithInstance(cluster.Name).WithManagedBy(),
Spec: v1.PodDisruptionBudgetSpec{
MinAvailable: cluster.Spec.PodDisruptionBudgetTemplate.Spec.MinAvailable,
MaxUnavailable: cluster.Spec.PodDisruptionBudgetTemplate.Spec.MaxUnavailable,
Selector: &metav1.LabelSelector{
MatchLabels: NewLabelsBuilder().WithName().WithInstance(cluster.Name).WithManagedBy(),
},
UnhealthyPodEvictionPolicy: ptr.To(v1.IfHealthyBudget),
},
UnhealthyPodEvictionPolicy: ptr.To(v1.IfHealthyBudget),
}
// if both are nil, calculate minAvailable based on number of replicas in the cluster
if pdb.Spec.MinAvailable == nil && pdb.Spec.MaxUnavailable == nil {
pdb.Spec.MinAvailable = ptr.To(intstr.FromInt32(int32(cluster.CalculateQuorumSize())))
}

ctx, err = contextWithGVK(ctx, pdb, rclient.Scheme())
if err != nil {
return err
}
log.Debug(ctx, "pdb spec generated", "spec", pdb.Spec)

if err := ctrl.SetControllerReference(cluster, pdb, rclient.Scheme()); err != nil {
if err = ctrl.SetControllerReference(cluster, pdb, rclient.Scheme()); err != nil {
return fmt.Errorf("cannot set controller reference: %w", err)
}

Expand Down
19 changes: 19 additions & 0 deletions internal/log/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,20 @@ type Parameters struct {
Development bool
}

// Setup initializes the logger and returns a new context with the logger attached.
// The logger is configured based on the provided Parameters. The encoder and writer
// are selected based on the Development flag. The LogLevel parameter determines the
// log level of the logger. The StacktraceLevel parameter determines the log level at
// which a stack trace is added to log entries.
// The function does not modify the original context.
//
// Example usage:
//
// ctx := Setup(context.Background(), Parameters{
// LogLevel: "debug",
// StacktraceLevel: "error",
// Development: true,
// })
func Setup(ctx context.Context, p Parameters) context.Context {
encoderConfig := zapcore.EncoderConfig{
MessageKey: "message",
Expand All @@ -74,22 +88,27 @@ func Setup(ctx context.Context, p Parameters) context.Context {
return logr.NewContextWithSlogLogger(ctx, l)
}

// Info logs an informational message with optional key-value pairs.
func Info(ctx context.Context, msg string, keysAndValues ...interface{}) {
logr.FromContextAsSlogLogger(ctx).With(keysAndValues...).Info(msg)
}

// Debug logs a debug message with optional key-value pairs.
func Debug(ctx context.Context, msg string, keysAndValues ...interface{}) {
logr.FromContextAsSlogLogger(ctx).With(keysAndValues...).Debug(msg)
}

// Warn logs a warning message with optional key-value pairs.
func Warn(ctx context.Context, msg string, keysAndValues ...interface{}) {
logr.FromContextAsSlogLogger(ctx).With(keysAndValues...).Warn(msg)
}

// Error logs an error message with optional key-value pairs.
func Error(ctx context.Context, err error, msg string, keysAndValues ...interface{}) {
logr.FromContextAsSlogLogger(ctx).With(keysAndValues...).Error(msg, slog.Any("error", err))
}

// WithValues adds additional key-value pairs to the context's logger.
func WithValues(ctx context.Context, keysAndValues ...interface{}) context.Context {
return logr.NewContextWithSlogLogger(ctx, logr.FromContextAsSlogLogger(ctx).With(keysAndValues...))
}
12 changes: 12 additions & 0 deletions internal/signal/signal.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,18 @@ import (
"os/signal"
)

// NotifyContext returns a derived context from the parent context with a cancel
// function. It sets up a signal channel to listen for the specified signals and
// cancels the context when any of those signals are received. If a second signal
// is received, it exits the program with a status code of 1.
//
// Example usage:
//
// ctx := signal.NotifyContext(parentContext, os.Interrupt, syscall.SIGTERM)
// go func() {
// <-ctx.Done()
// // Perform cleanup or other tasks before exiting
// }()
func NotifyContext(parent context.Context, signals ...os.Signal) context.Context {
ctx, cancel := context.WithCancel(parent)
c := make(chan os.Signal, 2)
Expand Down

0 comments on commit 0782ef6

Please sign in to comment.