Skip to content

OCPBUGS-20321: pkg/cvo/sync_worker: Always enable the DeploymentConfig capability #981

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

Merged
openshift-ci merged 1 commit into from
Oct 12, 2023
Merged

OCPBUGS-20321: pkg/cvo/sync_worker: Always enable the DeploymentConfig capability #981

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
6 changes: 6 additions & 0 deletions pkg/cvo/sync_worker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,12 @@ func (w *SyncWorker) syncPayload(ctx context.Context, work *SyncWork,
if w.payload != nil {
implicitlyEnabledCaps = payload.GetImplicitlyEnabledCapabilities(payloadUpdate.Manifests, w.payload.Manifests,
work.Capabilities)
if strings.HasPrefix(payloadUpdate.Release.Version, "4.14.") {
deploymentConfig := configv1.ClusterVersionCapability("DeploymentConfig")
if _, ok := work.Capabilities.EnabledCapabilities[deploymentConfig]; !ok && !capability.Contains(implicitlyEnabledCaps, deploymentConfig) {
implicitlyEnabledCaps = append(implicitlyEnabledCaps, deploymentConfig)
Copy link
Member

@LalatenduMohanty LalatenduMohanty Oct 12, 2023

Choose a reason for hiding this comment

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

@wking The issue is we have declared a capability but there are no associated manifests which has this capability. We can have this issue in future as well. Can we declare a generic code structure for this? A method which can return capability names which should be enabled when no manifests has the capability. I would expect the code structure to be present in master and then we should back-port this appropriately.

Copy link
Contributor

@bparees bparees Oct 12, 2023

Choose a reason for hiding this comment

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

we can assess a refactor in the future but right now we need this in a 4.13.z asap because users will need to upgrade to a 4.13.z containing this logic before they can upgrade to 4.14.0 (otherwise 4.13 clusters that have a static set of caps enabled in 4.13 will have the deploymentconfig cap disabled when they upgrade to 4.14.0)

Copy link
Member

@LalatenduMohanty LalatenduMohanty Oct 12, 2023

Choose a reason for hiding this comment

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

@bparees ack. Will add the labels

Copy link
Member Author

@wking wking Oct 25, 2023

Choose a reason for hiding this comment

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

I've created OTA-1040 trying to feel out a long-term plan for this kind of thing.

}
}
}
w.payload = payloadUpdate
msg = fmt.Sprintf("Payload loaded version=%q image=%q architecture=%q", desired.Version, desired.Image,
Expand Down