Add azure images by lobziik · Pull Request #46 · openshift/cluster-cloud-controller-manager-operator

lobziik · 2021-05-20T14:11:16Z

Add azure images to images configmap and image-reference file

JoelSpeed · 2021-05-20T14:19:33Z

I spoke with @Danil-Grigorev yesterday, would it be better to have a single image for Azure but with the two binaries in it? This would make the image substitution easier IIUC. This is something we have done with other things like spot termination handlers as part of MAO

lobziik · 2021-05-20T14:45:07Z

Agree, it would be easier. I'm mostly doubting about delivering ccm binary to worker nodes... Don't think that it might cause big issues but would prefer to keep it separate. Also building windows binary in future might require special treatment, but I'm not sure there.

For additional context:
upstream manifests for node-manager: https://github.com/openshift/cloud-provider-azure/blob/master/examples/out-of-tree/cloud-node-manager.yaml

lobziik · 2021-05-20T14:52:58Z

This might be another thing kubernetes-sigs/cloud-provider-azure#580 - we might want to use different images for different node types (e.g arm)

Danil-Grigorev · 2021-05-20T16:22:18Z

Agree, it would be easier. I'm mostly doubting about delivering ccm binary to worker nodes... Don't think that it might cause big issues but would prefer to keep it separate. Also building windows binary in future might require special treatment, but I'm not sure there.

For additional context:
upstream manifests for node-manager: https://github.com/openshift/cloud-provider-azure/blob/master/examples/out-of-tree/cloud-node-manager.yaml

https://coreos.slack.com/archives/CM4ERHBJS/p1620994660139000 сс @lobziik We are not required to run azure for windows nodes yet.

Danil-Grigorev · 2021-06-03T13:51:53Z

 type OperatorConfig struct {
 	ManagedNamespace string
 	ControllerImage  string
+	CloudNodeImage   string


I think this one will be better named CloudNodeControllerImage

lobziik · 2021-06-03T13:53:18Z

Suggest to rewiew and merge it for now as well as #45

Gonna introduce finished refactoring with interface (https://github.com/lobziik/cluster-cloud-controller-manager-operator/pull/1) as follow up, would be way more convenient for development and review.

Danil-Grigorev

/approve0

Danil-Grigorev · 2021-06-03T14:23:24Z

/lgtm
/approve

openshift-bot · 2021-06-03T14:29:49Z

/retest

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

openshift-bot · 2021-06-03T15:05:44Z

/retest

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

elmiko

/lgtm

openshift-ci · 2021-06-03T15:49:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Danil-Grigorev, 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:

~~OWNERS~~ [Danil-Grigorev,elmiko]

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

openshift-bot · 2021-06-03T17:41:42Z

/retest

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

openshift-bot · 2021-06-03T18:29:41Z

/retest

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

openshift-bot · 2021-06-03T20:54:36Z

/retest

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

openshift-bot · 2021-06-03T21:06:37Z

/retest

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

openshift-bot · 2021-06-03T23:06:36Z

/retest

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

openshift-bot · 2021-06-04T00:30:38Z

/retest

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

openshift-bot · 2021-06-04T01:06:37Z

/retest

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

openshift-ci · 2021-06-04T02:14:12Z

@lobziik: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws-ccm	`98d2a83`	link	`/test e2e-aws-ccm`

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-bot · 2021-06-04T04:06:37Z

/retest

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

openshift-bot · 2021-06-04T04:18:43Z

/retest

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

openshift-ci Bot requested review from JoelSpeed and mandre May 20, 2021 14:11

add azure images

10a62c7

Danil-Grigorev reviewed May 20, 2021

View reviewed changes

Comment thread manifests/0000_26_cloud-controller-manager-operator_01_images.configmap.yaml

lobziik changed the title ~~Add azure images~~ WIP: Add azure images May 20, 2021

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 May 20, 2021

add cloud node manager image and daemonset substitution

95e04e9

lobziik requested a review from Danil-Grigorev May 21, 2021 14:18

Danil-Grigorev reviewed May 24, 2021

View reviewed changes

Comment thread pkg/config/config.go

Comment thread pkg/substitution/substitution.go

Comment thread pkg/substitution/substitution.go Outdated

add missed tests

98d2a83

lobziik requested a review from Danil-Grigorev May 25, 2021 11:48

lobziik changed the title ~~WIP: Add azure images~~ Add azure images Jun 3, 2021

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 Jun 3, 2021

Danil-Grigorev reviewed Jun 3, 2021

View reviewed changes

Danil-Grigorev approved these changes Jun 3, 2021

View reviewed changes

openshift-ci Bot assigned Danil-Grigorev Jun 3, 2021

openshift-ci Bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 3, 2021

elmiko approved these changes Jun 3, 2021

View reviewed changes

openshift-ci Bot assigned elmiko Jun 3, 2021

openshift-merge-robot merged commit be2215a into openshift:master Jun 4, 2021

Uh oh!

Conversation

lobziik commented May 20, 2021

Uh oh!

JoelSpeed commented May 20, 2021

Uh oh!

lobziik commented May 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lobziik commented May 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Danil-Grigorev commented May 20, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Danil-Grigorev Jun 3, 2021

Choose a reason for hiding this comment

Uh oh!

lobziik commented Jun 3, 2021

Uh oh!

Danil-Grigorev left a comment

Choose a reason for hiding this comment

Uh oh!

Danil-Grigorev commented Jun 3, 2021

Uh oh!

openshift-bot commented Jun 3, 2021

Uh oh!

openshift-bot commented Jun 3, 2021

Uh oh!

elmiko left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Jun 3, 2021

Uh oh!

openshift-bot commented Jun 3, 2021

Uh oh!

openshift-bot commented Jun 3, 2021

Uh oh!

openshift-bot commented Jun 3, 2021

Uh oh!

openshift-bot commented Jun 3, 2021

Uh oh!

openshift-bot commented Jun 3, 2021

Uh oh!

openshift-bot commented Jun 4, 2021

Uh oh!

openshift-bot commented Jun 4, 2021

Uh oh!

openshift-ci Bot commented Jun 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-bot commented Jun 4, 2021

Uh oh!

openshift-bot commented Jun 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lobziik commented May 20, 2021 •

edited

Loading

lobziik commented May 20, 2021 •

edited

Loading

openshift-ci Bot commented Jun 4, 2021 •

edited

Loading