Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OCPCLOUD-2642: Include ASO CRDs for CAPZ installation #322

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nrb
Copy link

@nrb nrb commented Oct 29, 2024

This PR reverts the exclusion of ASO CRDs.

TODO:

@openshift-ci-robot
Copy link

openshift-ci-robot commented Oct 29, 2024

@nrb: This pull request references OCPCLOUD-2642 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

This PR reverts the exclusion of ASO CRDs.

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 openshift-eng/jira-lifecycle-plugin repository.

Copy link

openshift-ci bot commented Oct 29, 2024

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

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 29, 2024
@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 Oct 29, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Oct 29, 2024

@nrb: This pull request references OCPCLOUD-2642 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

This PR reverts the exclusion of ASO CRDs.

TODO:

  • Include ASO CRDs in kustomization
  • Include ASO CRDs in component ConfigMap

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 openshift-eng/jira-lifecycle-plugin repository.

Copy link
Member

@damdo damdo left a comment

Choose a reason for hiding this comment

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

Looks reasonable up until now 👍

@openshift-ci-robot
Copy link

openshift-ci-robot commented Oct 29, 2024

@nrb: This pull request references OCPCLOUD-2642 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

This PR reverts the exclusion of ASO CRDs.

TODO:

  • Include ASO CRDs in kustomization
  • Include ASO CRDs in component ConfigMap
  • Disable CRD management in ASO Deployment.

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 openshift-eng/jira-lifecycle-plugin repository.

@nrb
Copy link
Author

nrb commented Oct 29, 2024

@damdo @RadekManak Since the diff's too big for GitHub to render, how do we go about editing the arguments on the ASO Deployment? Use a Kustomize file to layer over the defaults?

Currently, it is

apiVersion: apps/v1
kind: Deployment
metadata:
  creationTimestamp: null
  labels:
    app.kubernetes.io/name: azure-service-operator
    app.kubernetes.io/version: v2.6.0
    cluster.x-k8s.io/provider: infrastructure-azure
    clusterctl.cluster.x-k8s.io: ""
    control-plane: controller-manager
  name: azureserviceoperator-controller-manager
  namespace: openshift-cluster-api
spec:
  replicas: 1
  selector:
    matchLabels:
      control-plane: controller-manager
  strategy: {}
  template:
    metadata:
      annotations:
        kubectl.kubernetes.io/default-container: manager
        target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}'
      creationTimestamp: null
      labels:
        aadpodidbinding: aso-manager-binding
        app.kubernetes.io/name: azure-service-operator
        app.kubernetes.io/version: v2.6.0
        control-plane: controller-manager
    spec:
      containers:
      - args:
        - --metrics-addr=:8080
        - --health-addr=:8081
        - --enable-leader-election
        - --v=2
        - --crd-pattern=${ADDITIONAL_ASO_CRDS:= }
        - --webhook-port=9443
        - --webhook-cert-dir=/tmp/k8s-webhook-server/serving-certs

From the help of the aso controller binary:

  -crd-management string
        Instructs the operator on how it should manage the Custom Resource Definitions. One of 'auto', 'none' (default "auto")
  -crd-pattern string
        Install these CRDs. CRDs already in the cluster will also always be upgraded.

So, ideally, we'd omit -crd-pattern entirely and add -crd-management=none.

@openshift-ci-robot
Copy link

openshift-ci-robot commented Oct 30, 2024

@nrb: This pull request references OCPCLOUD-2642 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

This PR reverts the exclusion of ASO CRDs.

TODO:

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 openshift-eng/jira-lifecycle-plugin repository.

@nrb
Copy link
Author

nrb commented Oct 30, 2024

I figured out an approach to updating the container arguments, however it relies on openshift/cluster-capi-operator#228 being present in manifests-gen before this can merge.

Copy link
Member

@damdo damdo left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me 👍

@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 6, 2024

@nrb: This pull request references OCPCLOUD-2642 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

This PR reverts the exclusion of ASO CRDs.

TODO:

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 openshift-eng/jira-lifecycle-plugin repository.

@nrb nrb marked this pull request as ready for review November 6, 2024 18:59
@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 Nov 6, 2024
@openshift-ci openshift-ci bot requested review from damdo and JoelSpeed November 6, 2024 19:00
@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 8, 2024

@nrb: This pull request references OCPCLOUD-2642 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

This PR reverts the exclusion of ASO CRDs.

TODO:

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 openshift-eng/jira-lifecycle-plugin repository.

@damdo
Copy link
Member

damdo commented Nov 9, 2024

/retest

2 similar comments
@damdo
Copy link
Member

damdo commented Nov 11, 2024

/retest

@bryan-cox
Copy link
Member

/retest

@bryan-cox
Copy link
Member

/test e2e-azure-serial

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 18, 2024
Copy link

openshift-ci bot commented Dec 18, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from nrb. For more information see the Code Review Process.

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

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
Copy link

openshift-ci-robot commented Dec 18, 2024

@nrb: This pull request references OCPCLOUD-2642 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

In response to this:

This PR reverts the exclusion of ASO CRDs.

TODO:

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 18, 2024
@nrb nrb changed the title OCPCLOUD-2642: Generate ASO CRDs for CAPZ installation OCPCLOUD-2642: Include ASO CRDs for CAPZ installation Dec 18, 2024
Copy link

openshift-ci bot commented Dec 19, 2024

@nrb: 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/okd-scos-e2e-aws-ovn da93f62 link false /test okd-scos-e2e-aws-ovn
ci/prow/e2e-azure-serial da93f62 link true /test e2e-azure-serial
ci/prow/e2e-azure-techpreview da93f62 link false /test e2e-azure-techpreview

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

@nrb
Copy link
Author

nrb commented Dec 19, 2024

While the tests are passing here, the Deployment isn't able to actually create pods because h the image isn't in payload quite yet - that happens in openshift/cluster-capi-operator#235.

So we'll need that PR to merge before we can have a valid test for this behavior.

            "status": {
                "conditions": [
                    {
                        "lastTransitionTime": "2024-12-18T21:22:08Z",
                        "lastUpdateTime": "2024-12-18T21:22:08Z",
                        "message": "Deployment does not have minimum availability.",
                        "reason": "MinimumReplicasUnavailable",
                        "status": "False",
                        "type": "Available"
                    },
                    {
                        "lastTransitionTime": "2024-12-18T21:32:09Z",
                        "lastUpdateTime": "2024-12-18T21:32:09Z",
                        "message": "ReplicaSet \"azureserviceoperator-controller-manager-75678d9f88\" has timed out progressing.",
                        "reason": "ProgressDeadlineExceeded",
                        "status": "False",
                        "type": "Progressing"
                    }
                ],

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants