Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 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
22 changes: 22 additions & 0 deletions pkg/controllers/installer/probes.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ func allProbes() []*probing.GroupKindSelector {
return []*probing.GroupKindSelector{
crdEstablishedProbe(),
deploymentAvailableProbe(),
compatibilityRequirementAdmittedProbe(),
compatibilityRequirementCompatibleProbe(),
Comment thread
theobarberbany marked this conversation as resolved.
}
}

Expand Down Expand Up @@ -88,6 +90,26 @@ func probeSucceededPredicate(probes ...*probing.GroupKindSelector) predicate.Pre
}
}

// compatibilityRequirementAdmittedProbe checks that a CompatibilityRequirement
// has the Admitted condition set to True. This confirms the validating webhook
// is in place to guard against future incompatible CRD updates.
func compatibilityRequirementAdmittedProbe() *probing.GroupKindSelector {
return &probing.GroupKindSelector{
GroupKind: schema.GroupKind{Group: "apiextensions.openshift.io", Kind: "CompatibilityRequirement"},
Prober: &probing.ConditionProbe{Type: "Admitted", Status: "True"},
}
}

// compatibilityRequirementCompatibleProbe checks that a CompatibilityRequirement
// has the Compatible condition set to True. This confirms the current CRD
// satisfies the compatibility contract.
func compatibilityRequirementCompatibleProbe() *probing.GroupKindSelector {
return &probing.GroupKindSelector{
GroupKind: schema.GroupKind{Group: "apiextensions.openshift.io", Kind: "CompatibilityRequirement"},
Prober: &probing.ConditionProbe{Type: "Compatible", Status: "True"},
}
}

// noGenerationPredicate returns a predicate that passes all update events for
// objects that don't have generation tracking (e.g., ConfigMaps, Secrets).
// Objects with generation tracking have it initialised to 1 on creation, so
Expand Down
5 changes: 4 additions & 1 deletion pkg/controllers/installer/revision_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,10 @@ 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.
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,
revisiongenerator.WithObjectCollectors(r.collectObjects),
revisiongenerator.WithUnmanagedCRDs(apiRev.UnmanagedCustomResourceDefinitions),
)
if err != nil {
err = fmt.Errorf("error creating installer revision from API revision %s: %w", apiRev.Name, reconcile.TerminalError(err))
}
Expand Down
25 changes: 16 additions & 9 deletions pkg/controllers/revision/revision_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,7 @@ func (r *RevisionController) Reconcile(ctx context.Context, _ ctrl.Request) (ctr
}

func (r *RevisionController) reconcile(ctx context.Context, log logr.Logger) operatorstatus.ReconcileResult {
// Generate a desired revision from the current state
desiredRevision, result := r.generateDesiredRevision(ctx)
if result != nil {
return *result
}

// Get ClusterAPI singleton
// Get ClusterAPI singleton first — generateDesiredRevision needs spec fields.

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.

This is such a clanker comment 😅

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yup - welcome to the future ..

clusterAPI := &operatorv1alpha1.ClusterAPI{}
if err := r.Get(ctx, client.ObjectKey{Name: clusterAPIName}, clusterAPI); err != nil {
if apierrors.IsNotFound(err) {
Expand All @@ -105,6 +99,16 @@ func (r *RevisionController) reconcile(ctx context.Context, log logr.Logger) ope
return opresult.Error(fmt.Errorf("fetching ClusterAPI: %w", err))
}

var unmanagedCRDs []string
if clusterAPI.Spec != nil {
unmanagedCRDs = clusterAPI.Spec.UnmanagedCustomResourceDefinitions
}

desiredRevision, result := r.generateDesiredRevision(ctx, unmanagedCRDs)
if result != nil {
return *result
}

// Create a reverse sorted, merged list of revisions. It will prepend the
// new revision if necessary. Note that the latest revision is always
// first, and there is guaranteed to be at least one revision.
Expand Down Expand Up @@ -134,7 +138,7 @@ func (r *RevisionController) reconcile(ctx context.Context, log logr.Logger) ope
return opresult.Success()
}

func (r *RevisionController) generateDesiredRevision(ctx context.Context) (revisiongenerator.RenderedRevision, *operatorstatus.ReconcileResult) {
func (r *RevisionController) generateDesiredRevision(ctx context.Context, unmanagedCRDs []string) (revisiongenerator.RenderedRevision, *operatorstatus.ReconcileResult) {
infra := &configv1.Infrastructure{}
if err := r.Get(ctx, client.ObjectKey{Name: infrastructureName}, infra); err != nil {
return nil, opresult.ErrorP(fmt.Errorf("fetching infrastructure: %w", err))
Expand All @@ -147,7 +151,10 @@ 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, revisiongenerator.WithManifestSubstitutions(r.manifestSubstitutions))
revision, err := revisiongenerator.NewRenderedRevision(providerComponents,
revisiongenerator.WithManifestSubstitutions(r.manifestSubstitutions),
revisiongenerator.WithUnmanagedCRDs(unmanagedCRDs),
)
if err != nil {
return nil, opresult.ErrorP(fmt.Errorf("error creating rendered revision: %w", err))
}
Expand Down
42 changes: 30 additions & 12 deletions pkg/revisiongenerator/revision.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ type renderedRevision struct {
components []*renderedComponent
contentID string
substitutions []operatorv1alpha1.ClusterAPIInstallerRevisionManifestSubstitution
unmanagedCRDs []string
}

var _ RenderedRevision = &renderedRevision{}
Expand Down Expand Up @@ -124,6 +125,7 @@ func newRenderedRevision(profiles []providerimages.ProviderImageManifests, opts
rev := &renderedRevision{
components: components,
substitutions: substitutionsFromMap(cfg.substitutions),
unmanagedCRDs: cfg.unmanagedCRDs,
}

if err := validateRenderedRevision(rev); err != nil {
Expand Down Expand Up @@ -236,9 +238,13 @@ func (r *installerRevision) ForInstall(_ string, _ int64) (InstallerRevision, er

// ToAPIRevision converts this revision to an API revision.
func (r *installerRevision) ToAPIRevision() (operatorv1alpha1.ClusterAPIInstallerRevision, error) {
apiComponents := make([]operatorv1alpha1.ClusterAPIInstallerComponent, len(r.components))
for i, component := range r.components {
apiComponents[i] = operatorv1alpha1.ClusterAPIInstallerComponent{
var apiComponents []operatorv1alpha1.ClusterAPIInstallerComponent
for _, component := range r.components {
if component.synthetic {
continue
}

apiComponents = append(apiComponents, operatorv1alpha1.ClusterAPIInstallerComponent{
Name: component.name,
ClusterAPIInstallerComponentSource: operatorv1alpha1.ClusterAPIInstallerComponentSource{
Type: operatorv1alpha1.InstallerComponentTypeImage,
Expand All @@ -247,7 +253,7 @@ func (r *installerRevision) ToAPIRevision() (operatorv1alpha1.ClusterAPIInstalle
Profile: component.profile,
},
},
}
})
}

contentID, err := r.ContentID()
Expand All @@ -256,11 +262,12 @@ func (r *installerRevision) ToAPIRevision() (operatorv1alpha1.ClusterAPIInstalle
}

return operatorv1alpha1.ClusterAPIInstallerRevision{
Name: r.revisionName,
Revision: r.revisionIndex,
ContentID: contentID,
ManifestSubstitutions: slices.Clone(r.substitutions),
Components: apiComponents,
Name: r.revisionName,
Revision: r.revisionIndex,
ContentID: contentID,
ManifestSubstitutions: slices.Clone(r.substitutions),
Components: apiComponents,
UnmanagedCustomResourceDefinitions: slices.Clone(r.unmanagedCRDs),
}, nil
}

Expand Down Expand Up @@ -289,6 +296,7 @@ func buildRevisionName(releaseVersion, contentID string, index int64) operatorv1
type revisionRenderConfig struct {
objectCollectors []RevisionObjectCollector
substitutions map[string]string
unmanagedCRDs []string
}

type revisionRenderOption func(*revisionRenderConfig)
Expand All @@ -304,6 +312,15 @@ func WithObjectCollectors(collectors ...RevisionObjectCollector) revisionRenderO
}
}

// WithUnmanagedCRDs sets the list of CRD names that should not be installed
// by the installer. These CRDs will be used to build CompatibilityRequirement
// objects in a synthetic component and filtered from their normal phases.
func WithUnmanagedCRDs(crds []string) revisionRenderOption {
return func(opts *revisionRenderConfig) {
opts.unmanagedCRDs = crds
}
}

// WithManifestSubstitutions adds envsubst-style substitutions that will be
// applied to manifests during rendering and recorded on the revision. When
// called multiple times, later values merge with and override earlier ones.
Expand Down Expand Up @@ -383,9 +400,10 @@ func NewInstallerRevisionFromAPI(
}

type renderedComponent struct {
name string
imageRef string
profile string
name string
imageRef string
profile string
synthetic bool

crds []unstructured.Unstructured
objects []unstructured.Unstructured
Expand Down