Skip to content

Conversation

@jottofar
Copy link
Contributor

@jottofar jottofar commented Feb 21, 2022

Implements initial, postinstall, support of static capabilities
per Jira OTA-567.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 21, 2022
@openshift-ci openshift-ci bot requested review from vrutkovs and wking February 21, 2022 16:17
@openshift-ci openshift-ci bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 21, 2022
@jottofar jottofar force-pushed the ota-567 branch 2 times, most recently from d9825a8 to abafa60 Compare February 28, 2022 21:42
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 28, 2022
continue
}
known[capability] = struct{}{}
config.Status.Capabilities.KnownCapabilities = append(config.Status.Capabilities.KnownCapabilities, capability)
Copy link
Member

Choose a reason for hiding this comment

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

Do we have reliable locking on this? With the status sync'er running in parallel, I'd expect at least something like building a slice in a local variable, and after completing the local array, slotting it in with:

config.Status.Capabilities.KnownCapabilities = knownCapabilities

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is old code and will be removed but the question of the need for a lock is valid.

The capabilities reside in get set SyncWork and are set in Update so are protected by the lock there. Just yesterday I added setting CapabilitiesStatus in SyncWorkerStatus also in Update here so it's available to syncStatus here.

@jottofar
Copy link
Contributor Author

jottofar commented Mar 1, 2022

/retitle Consume post install static spec capabilities

@openshift-ci openshift-ci bot changed the title WIP: Consume post install static spec capabilities Consume post install static spec capabilities Mar 1, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 1, 2022
@jottofar jottofar force-pushed the ota-567 branch 4 times, most recently from e850c3d to 999a747 Compare March 3, 2022 17:23
@jottofar
Copy link
Contributor Author

jottofar commented Mar 3, 2022

retitle WIP: Consume post install static spec capabilities

Get cluster version object earlier in startup needs to land first and unit test fixes in progress.

@jottofar jottofar force-pushed the ota-567 branch 3 times, most recently from 711c695 to 662bb9a Compare March 3, 2022 19:10
@jottofar
Copy link
Contributor Author

jottofar commented Mar 3, 2022

/retitle WIP: Consume post install static spec capabilities

@openshift-ci openshift-ci bot changed the title Consume post install static spec capabilities WIP: Consume post install static spec capabilities Mar 3, 2022
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 3, 2022
@jottofar
Copy link
Contributor Author

jottofar commented Mar 4, 2022

/test e2e-agnostic

@jottofar
Copy link
Contributor Author

/retitle Consume post install static spec capabilities

@openshift-ci openshift-ci bot changed the title WIP: Consume post install static spec capabilities Consume post install static spec capabilities Mar 11, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 11, 2022
}
actions = client.Actions()
if len(actions) != 1 {
if len(actions) != 2 {
Copy link
Member

Choose a reason for hiding this comment

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

what is the second action here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update to ClusterVersion

Copy link
Member

Choose a reason for hiding this comment

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

Can we grow an expectUpdateStatus or some such to enforce that?

Implements initial, postinstall, support of static capabilities
per Jira OTA-567.
Copy link
Member

@wking wking left a comment

Choose a reason for hiding this comment

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

/lgtm

sandbox and PBD issues are orthogonal.

/override ci/prow/e2e-agnostic-upgrade

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 14, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 14, 2022

@wking: Overrode contexts on behalf of wking: ci/prow/e2e-agnostic-upgrade

Details

In response to this:

/lgtm

sandbox and PBD issues are orthogonal.

/override ci/prow/e2e-agnostic-upgrade

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 14, 2022

@jottofar: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 14, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jottofar, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 1699e32 into openshift:master Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants