Skip to content

Revert "Revert "Add images config map""#31

Merged
openshift-merge-robot merged 3 commits intoopenshift:masterfrom
Danil-Grigorev:revert-30-revert-28-images-config-map
Apr 20, 2021
Merged

Revert "Revert "Add images config map""#31
openshift-merge-robot merged 3 commits intoopenshift:masterfrom
Danil-Grigorev:revert-30-revert-28-images-config-map

Conversation

@Danil-Grigorev
Copy link
Copy Markdown

Reverts #30

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 29, 2021
@openshift-ci-robot
Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@Danil-Grigorev Danil-Grigorev marked this pull request as ready for review March 29, 2021 11:46
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 29, 2021
@Danil-Grigorev Danil-Grigorev force-pushed the revert-30-revert-28-images-config-map branch from 2f35cd2 to 4631475 Compare April 16, 2021 12:23
volumes:
- name: images
configMap:
defaultMode: 420
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.

is this the proper mode?

chmod 420 produces -r---w---- which doesn't look right to me.

- Fix potantial name clash in image stream tags
- Add missing configMap annotations
@Danil-Grigorev Danil-Grigorev force-pushed the revert-30-revert-28-images-config-map branch from 4631475 to 9caa0e9 Compare April 16, 2021 14:19
Copy link
Copy Markdown
Contributor

@elmiko elmiko left a comment

Choose a reason for hiding this comment

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

@Danil-Grigorev this looks fine to me, but i think we should update the image urls now since we are adding them

Comment thread hack/example-images.json Outdated
@@ -0,0 +1,4 @@
{
"cloudControllerManagerAWS": "registry.svc.ci.openshift.org/openshift:aws-cloud-controller-manager",
"cloudControllerManagerOpenStack": "registry.svc.ci.openshift.org/openshift:openstack-cloud-controller-manager"
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.

we should probably fix these up before committing. no reason to use the old url

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Right, let me adjust that. CI is now passing

images.json: >
{
"cloudControllerManagerAWS": "registry.svc.ci.openshift.org/openshift:aws-cloud-controller-manager",
"cloudControllerManagerOpenStack": "registry.svc.ci.openshift.org/openshift:openstack-cloud-controller-manager"
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.

same here

Comment thread manifests/image-references Outdated
- name: aws-cloud-controller-manager
from:
kind: DockerImage
name: registry.svc.ci.openshift.org/openshift:aws-cloud-controller-manager
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.

here too

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Copy Markdown
Contributor

@elmiko elmiko left a comment

Choose a reason for hiding this comment

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

thanks Danil!
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 16, 2021
@openshift-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: elmiko

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-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 16, 2021
@openshift-bot
Copy link
Copy Markdown

/retest

Please review the full test history for this PR and help us cut down flakes.

@Danil-Grigorev
Copy link
Copy Markdown
Author

/retest

@openshift-bot
Copy link
Copy Markdown

/retest

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Copy Markdown

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 941963f into openshift:master Apr 20, 2021
@Danil-Grigorev Danil-Grigorev deleted the revert-30-revert-28-images-config-map branch April 20, 2021 09:11
@Danil-Grigorev Danil-Grigorev mentioned this pull request Apr 30, 2021
1 task
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.

5 participants