openshift / api

Canonical location of the OpenShift API definition.
http://www.openshift.org
Apache License 2.0
95 stars 510 forks source link

NE-1794: Promote `eipAllocations` API in GA #1989

Closed miheer closed 1 month ago

miheer commented 1 month ago

Promote eipAllocations API in GA

openshift-ci-robot commented 1 month ago

@miheer: This pull request references NE-1794 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.17.0" version, but no target version was set.

In response to [this](https://github.com/openshift/api/pull/1989): >Promote `eipAllocations` API in GA Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fapi). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
openshift-ci[bot] commented 1 month ago

Hello @miheer! Some important instructions when contributing to openshift/api: API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

miheer commented 1 month ago

/retest

miheer commented 1 month ago

/retest

miheer commented 1 month ago

/retest

miheer commented 1 month ago

/retest

miheer commented 1 month ago

/retest

miheer commented 1 month ago

@JoelSpeed PTAL

melvinjoseph86 commented 1 month ago

/label qe-approved Tested with cluster bot image

melvinjoseph@mjoseph-mac Downloads % oc get clusterversion 
NAME      VERSION                                                   AVAILABLE   PROGRESSING   SINCE   STATUS
version   4.17.0-0.ci.test-2024-08-05-055832-ci-ln-z57xcht-latest   True        False         28m     Cluster version is 4.17.0-0.ci.test-2024-08-05-055832-ci-ln-z57xcht-latest
$ oc get featuregates.config.openshift.io cluster -oyaml
<......>
spec: {}
status:
  featureGates:
  - disabled:
<......>
    enabled:
    - name: SetEIPForNLBIngressController
openshift-ci-robot commented 1 month ago

@miheer: This pull request references NE-1794 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.17.0" version, but no target version was set.

In response to [this](https://github.com/openshift/api/pull/1989): >Promote `eipAllocations` API in GA Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fapi). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
miheer commented 1 month ago

/hold

JoelSpeed commented 1 month ago

@miheer What is your intention for this feature gate promotion, just looking at the hold you've put on?

I can see the verify is failing due to a lack of automated tests, but you have QE approval.

So we could merge this assuming CI goes green?

@melvinjoseph86 Just looking at your comment, have you tested the complete feature end to end? The promotion isn't about the gate moving, but checking that the feature that it gates is working as expected and ready for GA

miheer commented 1 month ago

@JoelSpeed I see the following behavior. I am not sure if this is a desired behavior.

After running make update I see operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-Default.crd.yaml operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-DevPreviewNoUpgrade.crd.yaml operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-TechPreviewNoUpgrade.crd.yaml gets deleted. From my understanding operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-Default.crd.yaml should stay ? I see this file in https://github.com/openshift/api/pull/1966

Because when I try to run make generate and make update in cluster-ingress-operator after bumping this API change I get the following error because of this script https://github.com/openshift/cluster-ingress-operator/blob/master/hack/update-generated-crd.sh#L14-#L32: GO111MODULE=on GOFLAGS=-mod=vendor go mod edit -replace=github.com/openshift/api=github.com/miheer/api@8b69b7aa71591223c0fd29e6dd021bd0292a48d8 make generate make update cp: vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-Default.crd.yaml: No such file or directory make: *** [crd] Error 1 (

miheer commented 1 month ago

/unhold

miheer commented 1 month ago

@JoelSpeed thanks for explaining me in slack!.

miheer commented 1 month ago

/retest

miheer commented 1 month ago

/retest

miheer commented 1 month ago

/retest

miheer commented 1 month ago

/retest

miheer commented 1 month ago

/retest

lihongan commented 1 month ago

@melvinjoseph86 Just looking at your comment, have you tested the complete feature end to end? The promotion isn't about the gate moving, but checking that the feature that it gates is working as expected and ready for GA

/remove-label qe-approved need a feature test before adding the label

openshift-ci-robot commented 1 month ago

@miheer: This pull request references NE-1794 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.17.0" version, but no target version was set.

In response to [this](https://github.com/openshift/api/pull/1989): >Promote `eipAllocations` API in GA Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fapi). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
melvinjoseph86 commented 1 month ago

@JoelSpeed now i completed the basic testing of the complete feature and giving the qe ack for the changes. /label qe-approved

melvinjoseph@mjoseph-mac Downloads % oc -n openshift-ingress-operator get ingresscontroller eip -oyaml
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
  annotations:
    ingress.operator.openshift.io/auto-delete-load-balancer: ""
  creationTimestamp: "2024-08-06T06:15:38Z"
  finalizers:
  - ingresscontroller.operator.openshift.io/finalizer-ingresscontroller
  generation: 10
  name: eip
  namespace: openshift-ingress-operator
  resourceVersion: "107404"
  uid: e6c658e7-316f-4bba-9bcc-2ae061d9d24c
spec:
  clientTLS:
    clientCA:
      name: ""
    clientCertificatePolicy: ""
  domain: hongli-aws.qe.devcluster.openshift.com
  endpointPublishingStrategy:
    loadBalancer:
      dnsManagementPolicy: Managed
      providerParameters:
        aws:
          networkLoadBalancer:
            eipAllocations:
            - eipalloc-0e4c93b8f166a8f08
            - eipalloc-053f6427452266a8a
            - eipalloc-06d4d316914d9a4fc
          type: NLB
        type: AWS
      scope: External
    type: LoadBalancerService
  httpCompression: {}
  httpEmptyRequestsPolicy: Respond
  httpErrorCodePages:
    name: ""
  tuningOptions:
    reloadInterval: 0s
  unsupportedConfigOverrides: null  
melvinjoseph@mjoseph-mac Downloads % oc get svc -n openshift-ingress router-eip  -oyaml
apiVersion: v1
kind: Service
metadata:
  annotations:
    service.beta.kubernetes.io/aws-load-balancer-eip-allocations: eipalloc-0e4c93b8f166a8f08,eipalloc-053f6427452266a8a,eipalloc-06d4d316914d9a4fc
<---snip--->

i will update the premerge task with the test case created from this later

openshift-ci-robot commented 1 month ago

@miheer: This pull request references NE-1794 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.17.0" version, but no target version was set.

In response to [this](https://github.com/openshift/api/pull/1989): >Promote `eipAllocations` API in GA Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fapi). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
JoelSpeed commented 1 month ago

/lgtm /override ci/prow/verify-crd-schema /override ci/prow/verify

openshift-ci[bot] commented 1 month ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoelSpeed, miheer

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

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/openshift/api/blob/master/OWNERS)~~ [JoelSpeed] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
openshift-ci[bot] commented 1 month ago

@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/verify, ci/prow/verify-crd-schema

In response to [this](https://github.com/openshift/api/pull/1989#issuecomment-2270714535): >/lgtm >/override ci/prow/verify-crd-schema >/override ci/prow/verify Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
miheer commented 1 month ago

/retest

miheer commented 1 month ago

/retest

miheer commented 1 month ago

/retest

openshift-ci[bot] commented 1 month ago

@miheer: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
openshift-bot commented 1 month ago

[ART PR BUILD NOTIFIER]

Distgit: ose-cluster-config-api This PR has been included in build ose-cluster-config-api-container-v4.18.0-202408061615.p0.g6b4a57e.assembly.stream.el9. All builds following this will include this PR.