Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
59 changes: 55 additions & 4 deletions pkg/controllers/installer/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
"github.com/onsi/gomega/types"
configv1 "github.com/openshift/api/config/v1"
operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -244,6 +246,13 @@ func setupProviderProfiles() {
// It uses revisiongenerator to compute the content ID, then writes via status update.
func addRevision(ctx context.Context, providerNames ...string) operatorv1alpha1.ClusterAPIInstallerRevision {
GinkgoHelper()
return addRevisionWithOpts(ctx, nil, providerNames...)
}

// addRevisionWithOpts is like addRevision but accepts render options (e.g. WithProxyConfig)
// so the content ID matches what the controller computes.
func addRevisionWithOpts(ctx context.Context, opts []revisiongenerator.RevisionRenderOption, providerNames ...string) operatorv1alpha1.ClusterAPIInstallerRevision {
GinkgoHelper()

// Get current ClusterAPI to determine revision index.
clusterAPI := &operatorv1alpha1.ClusterAPI{}
Expand All @@ -255,7 +264,7 @@ func addRevision(ctx context.Context, providerNames ...string) operatorv1alpha1.
profiles := lookupProfiles(providerNames...)

// Render the revision to compute the correct content ID.
rendered, err := revisiongenerator.NewRenderedRevision(profiles)
rendered, err := revisiongenerator.NewRenderedRevision(profiles, opts...)
Expect(err).NotTo(HaveOccurred())

revisionIndex := int64(len(clusterAPI.Status.Revisions) + 1)
Expand Down Expand Up @@ -304,7 +313,7 @@ func lookupProfiles(names ...string) []providerimages.ProviderImageManifests {
return profiles
}

// createFixtures creates ClusterAPI and ClusterOperator singletons.
// createFixtures creates ClusterAPI, ClusterOperator, and Proxy singletons.
func createFixtures(ctx context.Context) {
GinkgoHelper()

Expand All @@ -321,24 +330,35 @@ func createFixtures(ctx context.Context) {
Expect(cl.Create(ctx, clusterAPIObj)).To(Succeed())
cleanupObjs = append(cleanupObjs, clusterAPIObj)

proxyObj := &configv1.Proxy{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
}
Expect(cl.Create(ctx, proxyObj)).To(Succeed())
cleanupObjs = append(cleanupObjs, proxyObj)

clusterOperatorObj := &configv1.ClusterOperator{
ObjectMeta: metav1.ObjectMeta{Name: "cluster-api"},
}
Expect(cl.Create(ctx, clusterOperatorObj)).To(Succeed())
cleanupObjs = append(cleanupObjs, clusterOperatorObj)
}

// createFixturesWithoutClusterAPI creates only the ClusterOperator (not ClusterAPI).
// createFixturesWithoutClusterAPI creates only the ClusterOperator and Proxy (not ClusterAPI).
func createFixturesWithoutClusterAPI(ctx context.Context) {
GinkgoHelper()

proxyObj := &configv1.Proxy{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
}
Expect(cl.Create(ctx, proxyObj)).To(Succeed())

clusterOperatorObj := &configv1.ClusterOperator{
ObjectMeta: metav1.ObjectMeta{Name: "cluster-api"},
}
Expect(cl.Create(ctx, clusterOperatorObj)).To(Succeed())

DeferCleanup(func(ctx context.Context) {
deleteAndWait(ctx, clusterOperatorObj)
deleteAndWait(ctx, proxyObj, clusterOperatorObj)
})
}

Expand Down Expand Up @@ -408,6 +428,37 @@ func getRelatedObjects(ctx context.Context) []configv1.ObjectReference {
return co.Status.RelatedObjects
}

// makeDeploymentAvailable waits for the named Deployment to exist and then
// sets its Available condition to True. This is a common pattern when a test
// revision includes a Deployment that the controller probes.
func makeDeploymentAvailable(ctx context.Context, name, namespace string) {
GinkgoHelper()

deploy := &appsv1.Deployment{}
deploy.SetName(name)
deploy.SetNamespace(namespace)

Eventually(func() error {
return cl.Get(ctx, client.ObjectKeyFromObject(deploy), deploy)
}).
WithContext(ctx).
WithTimeout(defaultEventuallyTimeout).
Should(Succeed())

Eventually(kWithCtx(ctx).UpdateStatus(deploy, func() {
deploy.Status.Conditions = []appsv1.DeploymentCondition{
{
Type: appsv1.DeploymentAvailable,
Status: corev1.ConditionTrue,
Reason: "MinimumReplicasAvailable",
},
}
})).
WithContext(ctx).
WithTimeout(defaultEventuallyTimeout).
Should(Succeed())
}

// waitForRevision waits for the given revision to be applied and the controller
// to report Progressing=False.
func waitForRevision(ctx context.Context, revision operatorv1alpha1.RevisionName) {
Expand Down
17 changes: 16 additions & 1 deletion pkg/controllers/installer/installer_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,22 @@ func (c *InstallerController) reconcile(ctx context.Context, log logr.Logger) op
return opresult.WaitingOnExternal("ClusterAPI revisions")
}

revisionReconciler := newRevisionReconciler(c, log)
// Read cluster-wide proxy configuration
var renderOpts []revisiongenerator.RevisionRenderOption

proxy, err := util.GetProxy(ctx, c.client)
if err != nil {
return opresult.Error(fmt.Errorf("fetching proxy: %w", err))
}

if envVars := util.ProxyEnvVars(proxy); len(envVars) > 0 {
log.Info("Injecting proxy configuration into provider manifests",
"httpProxy", proxy.Status.HTTPProxy, "httpsProxy", proxy.Status.HTTPSProxy, "noProxy", proxy.Status.NoProxy)

renderOpts = append(renderOpts, revisiongenerator.WithProxyConfig(envVars))
}

revisionReconciler := newRevisionReconciler(c, log, renderOpts...)
reconciledRevision, messages, errs := revisionReconciler.reconcile(ctx, clusterAPI.Status.Revisions)
Comment on lines +222 to 238

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this causes the behaviour of the installer to change based on runtime behaviour. I'll explain more in the review comment.


// Write relatedObjects via non-SSA merge patch so the SSA conditions
Expand Down
71 changes: 71 additions & 0 deletions pkg/controllers/installer/installer_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"github.com/openshift/cluster-capi-operator/pkg/operatorstatus"
"github.com/openshift/cluster-capi-operator/pkg/revisiongenerator"
"github.com/openshift/cluster-capi-operator/pkg/test"
"github.com/openshift/cluster-capi-operator/pkg/util"
)

func getConfigMap(ctx context.Context, name string) (*corev1.ConfigMap, error) {
Expand Down Expand Up @@ -649,6 +650,76 @@ var _ = Describe("InstallerController", Serial, func() {
})
})

var _ = Describe("InstallerController proxy", Serial, func() {
BeforeEach(func(ctx context.Context) {
createFixtures(ctx)
}, defaultNodeTimeout)

AfterEach(func(ctx context.Context) {
emptyRevision := addEmptyRevision(ctx)
waitForRevision(ctx, emptyRevision.Name)
}, defaultNodeTimeout)

It("injects proxy env vars into deployed Deployments", func(ctx context.Context) {
// Configure proxy before adding revision.
proxy := &configv1.Proxy{}
Expect(cl.Get(ctx, client.ObjectKey{Name: "cluster"}, proxy)).To(Succeed())
proxy.Status = configv1.ProxyStatus{
HTTPProxy: "http://proxy:3128",
HTTPSProxy: "https://proxy:3129",
NoProxy: ".cluster.local",
}
Expect(cl.Status().Update(ctx, proxy)).To(Succeed())

// The revision content ID must match what the controller computes,
// which includes proxy injection into Deployment manifests.
proxyEnvVars := util.ProxyEnvVars(proxy)
renderOpts := []revisiongenerator.RevisionRenderOption{
revisiongenerator.WithProxyConfig(proxyEnvVars),
}

revision := addRevisionWithOpts(ctx, renderOpts, providerDeployment)
makeDeploymentAvailable(ctx, deploymentName, "default")
waitForRevision(ctx, revision.Name)

// Re-read the Deployment and verify proxy env vars.
deploy := &appsv1.Deployment{}
deploy.SetName(deploymentName)
deploy.SetNamespace("default")
Expect(cl.Get(ctx, client.ObjectKeyFromObject(deploy), deploy)).To(Succeed())

env := deploy.Spec.Template.Spec.Containers[0].Env
Expect(env).To(ContainElement(SatisfyAll(
HaveField("Name", "HTTP_PROXY"),
HaveField("Value", "http://proxy:3128"),
)))
Expect(env).To(ContainElement(SatisfyAll(
HaveField("Name", "HTTPS_PROXY"),
HaveField("Value", "https://proxy:3129"),
)))
Expect(env).To(ContainElement(SatisfyAll(
HaveField("Name", "NO_PROXY"),
HaveField("Value", ".cluster.local"),
)))
}, defaultNodeTimeout)

It("does not inject proxy env vars when proxy is empty", func(ctx context.Context) {
// Proxy is created with empty status by createFixtures.
revision := addRevision(ctx, providerDeployment)
makeDeploymentAvailable(ctx, deploymentName, "default")
waitForRevision(ctx, revision.Name)

deploy := &appsv1.Deployment{}
deploy.SetName(deploymentName)
deploy.SetNamespace("default")
Expect(cl.Get(ctx, client.ObjectKeyFromObject(deploy), deploy)).To(Succeed())

for _, ev := range deploy.Spec.Template.Spec.Containers[0].Env {
Expect(ev.Name).NotTo(BeElementOf("HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY"))
}
}, defaultNodeTimeout)
})

var _ = Describe("InstallerController without ClusterAPI", Serial, func() {
It("reports WaitingOnExternal when ClusterAPI does not exist", func(ctx context.Context) {
createFixturesWithoutClusterAPI(ctx)
Expand Down
10 changes: 8 additions & 2 deletions pkg/controllers/installer/revision_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,16 +109,18 @@ type revisionReconciler struct {
collectedNonNSObjects sets.Set[collectedObjectRef] // intermediate storage
crdGKResourceMapping map[schema.GroupKind]string // CRD GK → resource
relatedObjects sets.Set[configv1.ObjectReference] // kept for backward compatibility
renderOpts []revisiongenerator.RevisionRenderOption
}

func newRevisionReconciler(installerController *InstallerController, log logr.Logger) *revisionReconciler {
func newRevisionReconciler(installerController *InstallerController, log logr.Logger, renderOpts ...revisiongenerator.RevisionRenderOption) *revisionReconciler {
return &revisionReconciler{
InstallerController: installerController,
log: log,
gvks: sets.New[schema.GroupVersionKind](),
collectedNonNSObjects: sets.New[collectedObjectRef](),
crdGKResourceMapping: make(map[schema.GroupKind]string),
relatedObjects: sets.New[configv1.ObjectReference](),
renderOpts: renderOpts,
}
}

Expand All @@ -136,8 +138,12 @@ func (r *revisionReconciler) reconcile(ctx context.Context, revisions []operator

// Convert all API revisions upfront so that collectObjects (and thus
// relatedObjects) is fully populated before reconciliation begins.
opts := make([]revisiongenerator.RevisionRenderOption, 0, 1+len(r.renderOpts))
opts = append(opts, revisiongenerator.WithObjectCollectors(r.collectObjects))
opts = append(opts, r.renderOpts...)

converted := util.SliceMap(revisions, func(apiRev operatorv1alpha1.ClusterAPIInstallerRevision) convertedRevision {
rev, err := revisiongenerator.NewInstallerRevisionFromAPI(apiRev, r.providerProfiles, revisiongenerator.WithObjectCollectors(r.collectObjects))
rev, err := revisiongenerator.NewInstallerRevisionFromAPI(apiRev, r.providerProfiles, opts...)
if err != nil {
err = fmt.Errorf("error creating installer revision from API revision %s: %w", apiRev.Name, reconcile.TerminalError(err))
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/controllers/revision/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,13 @@ func createFixtures(ctx context.Context, opts ...fixturesOption) {
cleanupObjs = append(cleanupObjs, clusterAPI)
}

// Create Proxy singleton
proxyObj := &configv1.Proxy{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
}
Expect(cl.Create(ctx, proxyObj)).To(Succeed())
cleanupObjs = append(cleanupObjs, proxyObj)

// Create ClusterOperator singleton
clusterOperator = &configv1.ClusterOperator{
ObjectMeta: metav1.ObjectMeta{Name: "cluster-api"},
Expand Down
20 changes: 19 additions & 1 deletion pkg/controllers/revision/revision_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,22 @@ func (r *RevisionController) generateDesiredRevision(ctx context.Context) (revis
// Build ordered component list from provider metadata
providerComponents := r.buildComponentList(infra.Status.PlatformStatus.Type)

revision, err := revisiongenerator.NewRenderedRevision(providerComponents)
// Read cluster-wide proxy configuration
var opts []revisiongenerator.RevisionRenderOption

proxy, err := util.GetProxy(ctx, r.Client)
if err != nil {
return nil, opresult.ErrorP(fmt.Errorf("fetching proxy: %w", err))
}

if envVars := util.ProxyEnvVars(proxy); len(envVars) > 0 {
ctrl.LoggerFrom(ctx).Info("Injecting proxy configuration into provider manifests",
"httpProxy", proxy.Status.HTTPProxy, "httpsProxy", proxy.Status.HTTPSProxy, "noProxy", proxy.Status.NoProxy)

opts = append(opts, revisiongenerator.WithProxyConfig(envVars))
}

revision, err := revisiongenerator.NewRenderedRevision(providerComponents, opts...)
if err != nil {
return nil, opresult.ErrorP(fmt.Errorf("error creating rendered revision: %w", err))
}
Expand Down Expand Up @@ -301,6 +316,9 @@ func (r *RevisionController) SetupWithManager(mgr ctrl.Manager) error {
},
}),
).
Watches(&configv1.Proxy{},
handler.EnqueueRequestsFromMapFunc(toClusterAPI),
).
Complete(r)
if err != nil {
return fmt.Errorf("failed to create controller: %w", err)
Expand Down
Loading