Skip to content

Commit

Permalink
Consolidate reconciler config fields into ReconcilerConfig (#453)
Browse files Browse the repository at this point in the history 
Most reconcilers need the same three config fields: `ResourceDirectory`, `Platform`, and `DefaultProfile`. Instead of passing these fields separately, we now pass them via a `ReconcilerConfig` struct.

Signed-off-by: Marko Lukša <[email protected]>
  • Loading branch information
@luksa
luksa authored Oct 28, 2024
1 parent ec13475 commit 881fff7
Show file tree
Hide file tree
Showing 11 changed files with 130 additions and 91 deletions.
22 changes: 11 additions & 11 deletions cmd/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,15 @@ func main() {
var metricsAddr string
var probeAddr string
var configFile string
var resourceDirectory string
var logAPIRequests bool
var printVersion bool
var leaderElectionEnabled bool
var reconcilerCfg config.ReconcilerConfig

flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
flag.StringVar(&configFile, "config-file", "/etc/sail-operator/config.properties", "Location of the config file, propagated by k8s downward APIs")
flag.StringVar(&resourceDirectory, "resource-directory", "/var/lib/sail-operator/resources", "Where to find resources (e.g. charts)")
flag.StringVar(&reconcilerCfg.ResourceDirectory, "resource-directory", "/var/lib/sail-operator/resources", "Where to find resources (e.g. charts)")
flag.BoolVar(&logAPIRequests, "log-api-requests", false, "Whether to log each request sent to the Kubernetes API server")
flag.BoolVar(&printVersion, "version", printVersion, "Prints version information and exits")
flag.BoolVar(&leaderElectionEnabled, "leader-elect", true,
Expand Down Expand Up @@ -124,41 +125,40 @@ func main() {

chartManager := helm.NewChartManager(mgr.GetConfig(), os.Getenv("HELM_DRIVER"))

platform, err := config.DetectPlatform(mgr.GetConfig())
reconcilerCfg.Platform, err = config.DetectPlatform(mgr.GetConfig())
if err != nil {
setupLog.Error(err, "unable to detect platform")
os.Exit(1)
}

var defaultProfile string
if platform == config.PlatformOpenShift {
defaultProfile = "openshift"
if reconcilerCfg.Platform == config.PlatformOpenShift {
reconcilerCfg.DefaultProfile = "openshift"
} else {
defaultProfile = "default"
reconcilerCfg.DefaultProfile = "default"
}

err = istio.NewReconciler(mgr.GetClient(), mgr.GetScheme(), resourceDirectory, platform, defaultProfile).
err = istio.NewReconciler(reconcilerCfg, mgr.GetClient(), mgr.GetScheme()).
SetupWithManager(mgr)
if err != nil {
setupLog.Error(err, "unable to create controller", "controller", "Istio")
os.Exit(1)
}

err = remoteistio.NewReconciler(mgr.GetClient(), mgr.GetScheme(), resourceDirectory, platform, defaultProfile).
err = remoteistio.NewReconciler(reconcilerCfg, mgr.GetClient(), mgr.GetScheme()).
SetupWithManager(mgr)
if err != nil {
setupLog.Error(err, "unable to create controller", "controller", "RemoteIstio")
os.Exit(1)
}

err = istiorevision.NewReconciler(mgr.GetClient(), mgr.GetScheme(), resourceDirectory, chartManager).
err = istiorevision.NewReconciler(reconcilerCfg, mgr.GetClient(), mgr.GetScheme(), chartManager).
SetupWithManager(mgr)
if err != nil {
setupLog.Error(err, "unable to create controller", "controller", "IstioRevision")
os.Exit(1)
}

err = istiocni.NewReconciler(mgr.GetClient(), mgr.GetScheme(), resourceDirectory, chartManager, platform, defaultProfile).
err = istiocni.NewReconciler(reconcilerCfg, mgr.GetClient(), mgr.GetScheme(), chartManager).
SetupWithManager(mgr)
if err != nil {
setupLog.Error(err, "unable to create controller", "controller", "IstioCNI")
Expand Down
18 changes: 7 additions & 11 deletions controllers/istio/istio_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,16 @@ import (

// Reconciler reconciles an Istio object
type Reconciler struct {
ResourceDirectory string
Platform config.Platform
DefaultProfile string
Config config.ReconcilerConfig
client.Client
Scheme *runtime.Scheme
}

func NewReconciler(client client.Client, scheme *runtime.Scheme, resourceDir string, platform config.Platform, defaultProfile string) *Reconciler {
func NewReconciler(cfg config.ReconcilerConfig, client client.Client, scheme *runtime.Scheme) *Reconciler {
return &Reconciler{
ResourceDirectory: resourceDir,
Platform: platform,
DefaultProfile: defaultProfile,
Client: client,
Scheme: scheme,
Config: cfg,
Client: client,
Scheme: scheme,
}
}

Expand Down Expand Up @@ -111,8 +107,8 @@ func validate(istio *v1alpha1.Istio) error {
func (r *Reconciler) reconcileActiveRevision(ctx context.Context, istio *v1alpha1.Istio) error {
values, err := revision.ComputeValues(
istio.Spec.Values, istio.Spec.Namespace, istio.Spec.Version,
r.Platform, r.DefaultProfile, istio.Spec.Profile,
r.ResourceDirectory, getActiveRevisionName(istio))
r.Config.Platform, r.Config.DefaultProfile, istio.Spec.Profile,
r.Config.ResourceDirectory, getActiveRevisionName(istio))
if err != nil {
return err
}
Expand Down
26 changes: 18 additions & 8 deletions controllers/istio/istio_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ var (
)

func TestReconcile(t *testing.T) {
resourceDir := t.TempDir()
cfg := newReconcilerTestConfig(t)

t.Run("returns error when Istio version not set", func(t *testing.T) {
istio := &v1alpha1.Istio{
Expand All @@ -61,7 +61,7 @@ func TestReconcile(t *testing.T) {
cl := newFakeClientBuilder().
WithObjects(istio).
Build()
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, config.PlatformKubernetes, "")
reconciler := NewReconciler(cfg, cl, scheme.Scheme)

_, err := reconciler.Reconcile(ctx, istio)
if err == nil {
Expand Down Expand Up @@ -97,7 +97,9 @@ func TestReconcile(t *testing.T) {
WithStatusSubresource(&v1alpha1.Istio{}).
WithObjects(istio).
Build()
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, config.PlatformKubernetes, "invalid-profile")
cfg := newReconcilerTestConfig(t)
cfg.DefaultProfile = "invalid-profile"
reconciler := NewReconciler(cfg, cl, scheme.Scheme)

_, err := reconciler.Reconcile(ctx, istio)
if err == nil {
Expand Down Expand Up @@ -137,7 +139,7 @@ func TestReconcile(t *testing.T) {
},
}).
Build()
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, config.PlatformKubernetes, "")
reconciler := NewReconciler(cfg, cl, scheme.Scheme)

_, err := reconciler.Reconcile(ctx, istio)
if err == nil {
Expand Down Expand Up @@ -222,7 +224,7 @@ func TestValidate(t *testing.T) {
}

func TestDetermineStatus(t *testing.T) {
resourceDir := t.TempDir()
cfg := newReconcilerTestConfig(t)

generation := int64(100)

Expand Down Expand Up @@ -533,7 +535,7 @@ func TestDetermineStatus(t *testing.T) {
WithObjects(initObjs...).
WithInterceptorFuncs(interceptorFuncs).
Build()
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, config.PlatformKubernetes, "")
reconciler := NewReconciler(cfg, cl, scheme.Scheme)

status, err := reconciler.determineStatus(ctx, istio, tc.reconciliationErr)
if (err != nil) != tc.wantErr {
Expand All @@ -548,7 +550,7 @@ func TestDetermineStatus(t *testing.T) {
}

func TestUpdateStatus(t *testing.T) {
resourceDir := t.TempDir()
cfg := newReconcilerTestConfig(t)

generation := int64(100)
oneMinuteAgo := testtime.OneMinuteAgo()
Expand Down Expand Up @@ -734,7 +736,7 @@ func TestUpdateStatus(t *testing.T) {
WithObjects(initObjs...).
WithInterceptorFuncs(interceptorFuncs).
Build()
reconciler := NewReconciler(cl, scheme.Scheme, resourceDir, config.PlatformKubernetes, "")
reconciler := NewReconciler(cfg, cl, scheme.Scheme)

err := reconciler.updateStatus(ctx, istio, tc.reconciliationErr)
if (err != nil) != tc.wantErr {
Expand Down Expand Up @@ -915,3 +917,11 @@ func noWrites(t *testing.T) interceptor.Funcs {
},
}
}

func newReconcilerTestConfig(t *testing.T) config.ReconcilerConfig {
return config.ReconcilerConfig{
ResourceDirectory: t.TempDir(),
Platform: config.PlatformKubernetes,
DefaultProfile: "",
}
}
22 changes: 8 additions & 14 deletions controllers/istiocni/istiocni_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,24 +56,18 @@ const (

// Reconciler reconciles an IstioCNI object
type Reconciler struct {
ResourceDirectory string
Platform config.Platform
DefaultProfile string
client.Client
Config config.ReconcilerConfig
Scheme *runtime.Scheme
ChartManager *helm.ChartManager
}

func NewReconciler(
client client.Client, scheme *runtime.Scheme, resourceDir string, chartManager *helm.ChartManager, platform config.Platform, defaultProfile string,
) *Reconciler {
func NewReconciler(cfg config.ReconcilerConfig, client client.Client, scheme *runtime.Scheme, chartManager *helm.ChartManager) *Reconciler {
return &Reconciler{
ResourceDirectory: resourceDir,
Platform: platform,
DefaultProfile: defaultProfile,
Client: client,
Scheme: scheme,
ChartManager: chartManager,
Config: cfg,
Client: client,
Scheme: scheme,
ChartManager: chartManager,
}
}

Expand Down Expand Up @@ -153,7 +147,7 @@ func (r *Reconciler) installHelmChart(ctx context.Context, cni *v1alpha1.IstioCN

// apply userValues on top of defaultValues from profiles
mergedHelmValues, err := istiovalues.ApplyProfilesAndPlatform(
r.ResourceDirectory, cni.Spec.Version, r.Platform, r.DefaultProfile, cni.Spec.Profile, helm.FromValues(userValues))
r.Config.ResourceDirectory, cni.Spec.Version, r.Config.Platform, r.Config.DefaultProfile, cni.Spec.Profile, helm.FromValues(userValues))
if err != nil {
return fmt.Errorf("failed to apply profile: %w", err)
}
Expand All @@ -166,7 +160,7 @@ func (r *Reconciler) installHelmChart(ctx context.Context, cni *v1alpha1.IstioCN
}

func (r *Reconciler) getChartDir(cni *v1alpha1.IstioCNI) string {
return path.Join(r.ResourceDirectory, cni.Spec.Version, "charts", cniChartName)
return path.Join(r.Config.ResourceDirectory, cni.Spec.Version, "charts", cniChartName)
}

func applyImageDigests(cni *v1alpha1.IstioCNI, values *v1alpha1.CNIValues, config config.OperatorConfig) *v1alpha1.CNIValues {
Expand Down
21 changes: 16 additions & 5 deletions controllers/istiocni/istiocni_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ import (
)

func TestValidate(t *testing.T) {
cfg := newReconcilerTestConfig(t)

ns := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "istio-cni",
Expand Down Expand Up @@ -107,7 +109,7 @@ func TestValidate(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
g := NewWithT(t)
cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(tc.objects...).Build()
r := NewReconciler(cl, scheme.Scheme, "", nil, config.PlatformKubernetes, "")
r := NewReconciler(cfg, cl, scheme.Scheme, nil)

err := r.validate(context.TODO(), tc.cni)
if tc.expectErr == "" {
Expand Down Expand Up @@ -177,7 +179,7 @@ func newCondition(condType v1alpha1.IstioCNIConditionType, status metav1.Conditi
}

func TestDetermineReadyCondition(t *testing.T) {
resourceDir := t.TempDir()
cfg := newReconcilerTestConfig(t)

testCases := []struct {
name string
Expand Down Expand Up @@ -282,7 +284,7 @@ func TestDetermineReadyCondition(t *testing.T) {

cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(tt.clientObjects...).WithInterceptorFuncs(tt.interceptors).Build()

r := NewReconciler(cl, scheme.Scheme, resourceDir, nil, config.PlatformKubernetes, "")
r := NewReconciler(cfg, cl, scheme.Scheme, nil)

cni := &v1alpha1.IstioCNI{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -447,6 +449,8 @@ func TestApplyImageDigests(t *testing.T) {
}

func TestDetermineStatus(t *testing.T) {
cfg := newReconcilerTestConfig(t)

tests := []struct {
name string
reconcileErr error
Expand All @@ -462,9 +466,8 @@ func TestDetermineStatus(t *testing.T) {
}

ctx := context.TODO()
resourceDir := t.TempDir()
cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build()
r := NewReconciler(cl, scheme.Scheme, resourceDir, nil, config.PlatformKubernetes, "")
r := NewReconciler(cfg, cl, scheme.Scheme, nil)

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -497,3 +500,11 @@ func normalize(condition v1alpha1.IstioCNICondition) v1alpha1.IstioCNICondition
condition.LastTransitionTime = metav1.Time{}
return condition
}

func newReconcilerTestConfig(t *testing.T) config.ReconcilerConfig {
return config.ReconcilerConfig{
ResourceDirectory: t.TempDir(),
Platform: config.PlatformKubernetes,
DefaultProfile: "",
}
}
19 changes: 10 additions & 9 deletions controllers/istiorevision/istiorevision_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"github.com/go-logr/logr"
"github.com/istio-ecosystem/sail-operator/api/v1alpha1"
"github.com/istio-ecosystem/sail-operator/pkg/config"
"github.com/istio-ecosystem/sail-operator/pkg/constants"
"github.com/istio-ecosystem/sail-operator/pkg/enqueuelogger"
"github.com/istio-ecosystem/sail-operator/pkg/errlist"
Expand Down Expand Up @@ -67,17 +68,17 @@ const (
// Reconciler reconciles an IstioRevision object
type Reconciler struct {
client.Client
Scheme *runtime.Scheme
ResourceDirectory string
ChartManager *helm.ChartManager
Config config.ReconcilerConfig
Scheme *runtime.Scheme
ChartManager *helm.ChartManager
}

func NewReconciler(client client.Client, scheme *runtime.Scheme, resourceDir string, chartManager *helm.ChartManager) *Reconciler {
func NewReconciler(cfg config.ReconcilerConfig, client client.Client, scheme *runtime.Scheme, chartManager *helm.ChartManager) *Reconciler {
return &Reconciler{
Client: client,
Scheme: scheme,
ResourceDirectory: resourceDir,
ChartManager: chartManager,
Config: cfg,
Client: client,
Scheme: scheme,
ChartManager: chartManager,
}
}

Expand Down Expand Up @@ -181,7 +182,7 @@ func getReleaseName(rev *v1alpha1.IstioRevision) string {
}

func (r *Reconciler) getChartDir(rev *v1alpha1.IstioRevision) string {
return path.Join(r.ResourceDirectory, rev.Spec.Version, "charts", getChartName(rev))
return path.Join(r.Config.ResourceDirectory, rev.Spec.Version, "charts", getChartName(rev))
}

func getChartName(rev *v1alpha1.IstioRevision) string {
Expand Down
Loading

0 comments on commit 881fff7

Please sign in to comment.