Skip to content

Special case the global OpenShift image pull secret#3278

Merged
dhaus67 merged 3 commits intomasterfrom
cgorman-global-pull-secret
Oct 19, 2022
Merged

Special case the global OpenShift image pull secret#3278
dhaus67 merged 3 commits intomasterfrom
cgorman-global-pull-secret

Conversation

@connorgorman
Copy link
Contributor

@connorgorman connorgorman commented Oct 3, 2022

Description

This PR is an enhancement of #2572 and takes into account existing global pull secrets.

Openshift offers a global pull secret named pull-secret in the openshift-config namespace.

This pull secret can be used to give access to registries on a global scale, with the pull secret being rolled out to each nodes respective container runtime configuration.

The PR will make sure that the global pull secret will be added to the pull secrets for a specific image, irrespective of whether it's not matching in e.g. namespace.

Checklist

  • Investigated and inspected CI test results
    - [ ] Unit test and regression tests added
    - [ ] Evaluated and added CHANGELOG entry if required
    - [ ] Determined and documented upgrade steps
    - [ ] Documented user facing changes (create PR based on openshift/openshift-docs and merge into rhacs-docs)

Testing Performed

  • see CI.

@connorgorman connorgorman requested a review from dhaus67 October 3, 2022 00:26
@connorgorman
Copy link
Contributor Author

/test all

@openshift-ci
Copy link

openshift-ci bot commented Oct 3, 2022

@connorgorman: No presubmit jobs available for stackrox/stackrox@cgorman-image-integration-fix

Details

In response to this:

/test all

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.

@connorgorman connorgorman changed the base branch from cgorman-image-integration-fix to master October 3, 2022 01:05
@ghost
Copy link

ghost commented Oct 3, 2022

Images are ready for the commit at d9668da.

To use with deploy scripts, first export MAIN_IMAGE_TAG=3.72.x-344-gd9668daf5f.

@connorgorman connorgorman force-pushed the cgorman-global-pull-secret branch from 5fb921d to 91aeec7 Compare October 3, 2022 01:32
@connorgorman connorgorman added the pls-merge Indicates to the *reviewer* that the PR can be merged once tests pass and approval is granted label Oct 4, 2022
@misberner
Copy link
Contributor

@dhaus67 making sure you saw this? I can try to review it if things are not clear, just wanted to make sure it doesn't get lost

@dhaus67
Copy link
Contributor

dhaus67 commented Oct 6, 2022

@misberner: Most of the changes are from #2572 , I already reviewed that one prior.
If you have the time, it might be worth for you to have a look as well.

I'll refactor this PR to not include the changes from #2572 so its better to review, would appreciate a second look from you as well (preferably on both, if you got the time for it, the first one is quite big).

@dhaus67 dhaus67 force-pushed the cgorman-global-pull-secret branch from 91aeec7 to 5ffba36 Compare October 6, 2022 23:13
@dhaus67 dhaus67 changed the base branch from master to cgorman-image-integration-fix October 6, 2022 23:13
@dhaus67 dhaus67 requested a review from misberner October 6, 2022 23:14
return nil
}

func isOpenshiftGlobalRegistry(source *storage.ImageIntegration_Source) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func isOpenshiftGlobalRegistry(source *storage.ImageIntegration_Source) bool {
func isOpenshiftGlobalPullSecret(source *storage.ImageIntegration_Source) bool {

the pull secret has nothing to do with an OpenShift registry, at least not necessarily so

Base automatically changed from cgorman-image-integration-fix to master October 18, 2022 11:03
@dhaus67 dhaus67 force-pushed the cgorman-global-pull-secret branch from 26bc630 to d9668da Compare October 18, 2022 11:11
@openshift-ci
Copy link

openshift-ci bot commented Oct 18, 2022

@connorgorman: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/gke-postgres-qa-e2e-tests d9668da link false /test gke-postgres-qa-e2e-tests
ci/prow/gke-qa-e2e-tests d9668da link false /test gke-qa-e2e-tests

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.

@dhaus67
Copy link
Contributor

dhaus67 commented Oct 19, 2022

CI flakes are addressed with #3471 , so merging this.

@dhaus67 dhaus67 merged commit 5d2c7c3 into master Oct 19, 2022
@dhaus67 dhaus67 deleted the cgorman-global-pull-secret branch October 19, 2022 00:14
ivan-degtiarenko pushed a commit that referenced this pull request Nov 5, 2022
Co-authored-by: Daniel Haus <dhaus@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/central area/sensor pls-merge Indicates to the *reviewer* that the PR can be merged once tests pass and approval is granted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants