[WRKLDS-730] react to route-controller-manager refactoring by atiratree · Pull Request #2674 · openshift/hypershift

atiratree · 2023-06-09T20:38:27Z

What this PR does / why we need it:

split config from OCM and pass only relevant data
pass kubeconfig and pod name/namespace

This has to be merged before openshift/route-controller-manager#28

Which issue(s) this PR fixes
Fixes # https://issues.redhat.com/browse/WRKLDS-730

Checklist

Subject and description added to both, commit and PR.
Relevant issues have been referenced.
This change includes docs.
This change includes unit tests.

atiratree · 2023-06-12T12:14:07Z

/retest

atiratree · 2023-06-12T12:27:33Z

we are getting Error: unknown flag: --kubeconfig in the e2e for RCM https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_hypershift/2674/pull-ci-openshift-hypershift-main-e2e-aws/1667269965419384832/artifacts/e2e-aws/run-e2e/artifacts/TestUpgradeControlPlane_PreTeardownClusterDump/namespaces/e2e-clusters-gwnqn-example-gmnvf/core/pods/logs/openshift-route-controller-manager-5485778469-5w84x-route-controller-manager.log

so we need to merge changes in openshift/route-controller-manager#28 first and then react to the changes with this PR right away

atiratree · 2023-06-12T13:05:03Z

/test ?

openshift-ci · 2023-06-12T13:05:31Z

@atiratree: The following commands are available to trigger required jobs:

/test e2e-aws
/test images
/test unit
/test verify

The following commands are available to trigger optional jobs:

/test e2e-aws-4-12
/test e2e-aws-4-13
/test e2e-aws-metrics
/test e2e-conformance
/test e2e-ibmcloud-iks
/test e2e-ibmcloud-roks
/test e2e-kubevirt-aws-ovn

Use /test all to run the following jobs that were automatically triggered:

pull-ci-openshift-hypershift-main-e2e-aws
pull-ci-openshift-hypershift-main-e2e-kubevirt-aws-ovn
pull-ci-openshift-hypershift-main-images
pull-ci-openshift-hypershift-main-unit
pull-ci-openshift-hypershift-main-verify

Details

In response to this:

/test ?

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.

atiratree · 2023-06-12T13:08:59Z

let's see if it passes 4.13 job
/test e2e-aws-4-13

control-plane-operator/controllers/hostedcontrolplane/routecm/deployment.go

control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go

enxebre · 2023-06-19T08:44:01Z

control-plane-operator/controllers/hostedcontrolplane/routecm/config.go

+	configKey = "config.yaml"
+)
+
+func ReconcileOpenShiftRouteControllerManagerConfig(cm *corev1.ConfigMap, ownerRef config.OwnerRef, minTLSVersion string, cipherSuites []string, networkConfig *configv1.NetworkSpec) error {


can we please have a unit test for this?

added tests for both OCM and RCM

atiratree · 2023-07-03T16:02:05Z

@enxebre can you please take a look again? Also this has to be merged at the same exact time as openshift/route-controller-manager#28 since they are dependent on each other. And openshift/cluster-openshift-controller-manager-operator#295 is a prerequisite.

atiratree · 2023-07-03T18:47:30Z

/test verify

ardaguclu · 2023-07-10T07:35:36Z

@enxebre would you mind taking a look at this?. Thanks.

atiratree · 2023-07-20T09:54:11Z

/retest

atiratree · 2023-07-20T12:12:47Z

/retest

atiratree · 2023-07-20T15:42:15Z

/retest

atiratree · 2023-07-21T08:46:51Z

/retest

ingvagabund · 2023-07-24T09:09:28Z

/retest-required

atiratree · 2023-07-26T10:22:25Z

@enxebre the tests are passing, could you please do another review?

control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go

enxebre · 2023-08-01T10:20:02Z

control-plane-operator/controllers/hostedcontrolplane/routecm/config.go

+	if cm.Data == nil {
+		cm.Data = map[string]string{}
+	}
+	config := &openshiftcpv1.OpenShiftControllerManagerConfig{}


why do we need to deserialize this instead of just construct the struct, serialize and store it?

@enxebre I thought it was to potentially allow changes to the config by another actor (similar to OCM). But I can remove the deserialization part if that is not needed.

- split config from OCM and pass only relevant data - pass kubeconfig and pod name/namespace

…ncileOpenShiftControllerManagerConfig

netlify · 2023-08-01T10:31:26Z

✅ Deploy Preview for hypershift-docs ready!

Name	Link
🔨 Latest commit	`b8cec42`
🔍 Latest deploy log	https://app.netlify.com/sites/hypershift-docs/deploys/64c8ded58ee7760008d0944f
😎 Deploy Preview	https://deploy-preview-2674--hypershift-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

enxebre · 2023-08-01T10:38:23Z

/approve

openshift-ci · 2023-08-01T10:38:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: atiratree, enxebre

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~~ [enxebre]

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

enxebre · 2023-08-01T10:39:42Z

/label tide/merge-method-squash

openshift-ci · 2023-08-01T12:28:08Z

@atiratree: The following test 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/e2e-aws-4-13	`cfcf955`	link	false	`/test e2e-aws-4-13`

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.

atiratree · 2023-08-01T12:48:47Z

/test e2e-kubevirt-aws-ovn

sjenning · 2023-08-01T13:07:14Z

/lgtm

openshift-ci bot added the do-not-merge/needs-area label Jun 9, 2023

openshift-ci bot requested review from enxebre and sjenning June 9, 2023 20:38

openshift-ci bot added area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release and removed do-not-merge/needs-area labels Jun 9, 2023

atiratree force-pushed the rcm-updates branch from 2d0d623 to cfcf955 Compare June 12, 2023 13:03

enxebre reviewed Jun 19, 2023

View reviewed changes

control-plane-operator/controllers/hostedcontrolplane/routecm/deployment.go Outdated Show resolved Hide resolved

enxebre reviewed Jun 19, 2023

View reviewed changes

control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go Outdated Show resolved Hide resolved

enxebre reviewed Jun 19, 2023

View reviewed changes

atiratree force-pushed the rcm-updates branch from cfcf955 to 461c1f0 Compare July 3, 2023 15:57

atiratree mentioned this pull request Jul 19, 2023

[WRKLDS-730] add dummy flags needed for a switch to library-go server and ControlerCommand openshift/route-controller-manager#29

Merged

enxebre reviewed Aug 1, 2023

View reviewed changes

control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go Outdated Show resolved Hide resolved

enxebre reviewed Aug 1, 2023

View reviewed changes

react to route-controller-manager refactoring

7eb1b47

- split config from OCM and pass only relevant data - pass kubeconfig and pod name/namespace

add tests for ReconcileOpenShiftRouteControllerManagerConfig and Reco…

b8cec42

…ncileOpenShiftControllerManagerConfig

atiratree force-pushed the rcm-updates branch from 461c1f0 to b8cec42 Compare August 1, 2023 10:30

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 1, 2023

openshift-ci bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Aug 1, 2023

openshift-ci bot assigned sjenning Aug 1, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 1, 2023

openshift-merge-robot merged commit 1fc3c32 into openshift:main Aug 1, 2023

atiratree deleted the rcm-updates branch August 1, 2023 20:27

atiratree mentioned this pull request Aug 2, 2023

WRKLDS-730: use default /healthz path for readiness probe in OCM and RCM #2873

Merged

4 tasks

atiratree mentioned this pull request Nov 21, 2023

WRKLDS-925: remove duplicate passing of kubeconfig into route-controller-manager #3220

Merged

4 tasks

Conversation

atiratree commented Jun 9, 2023

Uh oh!

atiratree commented Jun 12, 2023

Uh oh!

atiratree commented Jun 12, 2023

Uh oh!

atiratree commented Jun 12, 2023

Uh oh!

openshift-ci bot commented Jun 12, 2023

Uh oh!

atiratree commented Jun 12, 2023

Uh oh!

Uh oh!

Uh oh!

enxebre Jun 19, 2023

Choose a reason for hiding this comment

Uh oh!

atiratree Jul 3, 2023

Choose a reason for hiding this comment

Uh oh!

atiratree commented Jul 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

atiratree commented Jul 3, 2023

Uh oh!

ardaguclu commented Jul 10, 2023

Uh oh!

atiratree commented Jul 20, 2023

Uh oh!

atiratree commented Jul 20, 2023

Uh oh!

atiratree commented Jul 20, 2023

Uh oh!

atiratree commented Jul 21, 2023

Uh oh!

ingvagabund commented Jul 24, 2023

Uh oh!

atiratree commented Jul 26, 2023

Uh oh!

Uh oh!

enxebre Aug 1, 2023

Choose a reason for hiding this comment

Uh oh!

atiratree Aug 1, 2023

Choose a reason for hiding this comment

Uh oh!

netlify bot commented Aug 1, 2023

✅ Deploy Preview for hypershift-docs ready!

Uh oh!

enxebre commented Aug 1, 2023

Uh oh!

openshift-ci bot commented Aug 1, 2023

Uh oh!

enxebre commented Aug 1, 2023

Uh oh!

openshift-ci bot commented Aug 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

atiratree commented Aug 1, 2023

Uh oh!

sjenning commented Aug 1, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

atiratree commented Jul 3, 2023 •

edited

Loading

openshift-ci bot commented Aug 1, 2023 •

edited

Loading