openshift
NE-1516: Adds a field in the cluster ingress config object and Ingress Controller to set EIP via installer and Ingress Controller
Adds a field in the cluster ingress config object and Ingress Controller to set EIP via installer and Ingress Controller
config/v1/types_ingress.go - Adds an API field eip-allocations in the cluster ingress config object whose value is set by the installer using install-config.yaml
operator/v1/types_ingress.go - Adds an API field eip-allocations in the Ingress Controller CR object using which the Ingress Operator scales an Ingress Controller with service type balancer whose annotation
service.beta.kubernetes.io/aws-load-balancer-eip-allocations is set by the value of the field eip-allocations of the Ingress Controller.
Epic: https://issues.redhat.com/browse/NE-1274
Story: https://issues.redhat.com/browse/NE-1516
@miheer: This pull request references NE-1516 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.16.0" version, but no target version was set.
In response to this:
Adds a field in the cluster ingress config object and Ingress Controller to set EIP via installer and Ingress Controller
config/v1/types_ingress.go- Adds an API fieldeip-allocationsin the cluster ingress config object whose value is set by the installer using install-config.yamloperator/v1/types_ingress.go- Adds an API fieldeip-allocationsin the Ingress Controller CR object using which the Ingress Operator scales an Ingress Controller with service type balancer whose annotationservice.beta.kubernetes.io/aws-load-balancer-eip-allocationsis set by the value of the fieldeip-allocationsof the Ingress Controller. Epic: https://issues.redhat.com/browse/NE-1274 Story: https://issues.redhat.com/browse/NE-1516
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.
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.
![openshift-ci[bot] avatar](https://avatars.githubusercontent.com/in/91280?v=4 "Published by a pub.dev verified publisher"){kind=small}
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: miheer Once this PR has been reviewed and has the lgtm label, please assign knobunc for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@miheer: This pull request references NE-1516 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.16.0" version, but no target version was set.
In response to this:
Adds a field in the cluster ingress config object and Ingress Controller to set EIP via installer and Ingress Controller
config/v1/types_ingress.go- Adds an API fieldeip-allocationsin the cluster ingress config object whose value is set by the installer using install-config.yaml
operator/v1/types_ingress.go- Adds an API fieldeip-allocationsin the Ingress Controller CR object using which the Ingress Operator scales an Ingress Controller with service type balancer whose annotation
service.beta.kubernetes.io/aws-load-balancer-eip-allocationsis set by the value of the fieldeip-allocationsof the Ingress Controller.Epic: https://issues.redhat.com/browse/NE-1274
Story: https://issues.redhat.com/browse/NE-1516
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.
@deads2k make update followed by make verify fails with the following:
Found untracked file 0000_10_config-operator_01_ingresses-Default.crd.yaml in payload CRD manifests. Please add the file to crd_globs in hack/update-payload-crds.sh.
The file is present in payload-manifests dir
% ls -la payload-manifests/crds/0000_10_config-operator_01_ingresses-Default.crd.yaml -rw-r--r-- 1 misalunk staff 33921 22 Apr 23:24 payload-manifests/crds/0000_10_config-operator_01_ingresses-Default.crd.yaml
However it is not present in config/v1/zz_generated.crd-manifests/
% ls -la config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-Default.crd.yaml ls: config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-Default.crd.yaml: No such file or directory misalunk@misalunk-mac api %
So, after manually copying
% cp payload-manifests/crds/0000_10_config-operator_01_ingresses-Default.crd.yaml config/v1/zz_generated.crd-manifests/
the error for above file is no longer present but happens for another file. Please check this
misalunk@misalunk-mac api % ./hack/verify-payload-crds.sh Verifying diff on 0000_03_config-operator_01_proxies.crd.yaml Verifying diff on 0000_10_config-operator_01_apiservers.crd.yaml Verifying diff on 0000_10_config-operator_01_authentications-Hypershift.crd.yaml Verifying diff on 0000_10_config-operator_01_authentications-SelfManagedHA-CustomNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_authentications-SelfManagedHA-Default.crd.yaml Verifying diff on 0000_10_config-operator_01_authentications-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_authentications-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_consoles.crd.yaml Verifying diff on 0000_10_config-operator_01_dnses.crd.yaml Verifying diff on 0000_10_config-operator_01_featuregates.crd.yaml Verifying diff on 0000_10_config-operator_01_imagecontentpolicies.crd.yaml Verifying diff on 0000_10_config-operator_01_imagedigestmirrorsets.crd.yaml Verifying diff on 0000_10_config-operator_01_images.crd.yaml Verifying diff on 0000_10_config-operator_01_imagetagmirrorsets.crd.yaml Verifying diff on 0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_infrastructures-Default.crd.yaml Verifying diff on 0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_ingresses-CustomNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_ingresses-Default.crd.yaml Verifying diff on 0000_10_config-operator_01_ingresses.crd.yaml Verifying diff on 0000_10_config-operator_01_networks-CustomNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_networks-Default.crd.yaml Verifying diff on 0000_10_config-operator_01_networks-DevPreviewNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_networks-TechPreviewNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_nodes.crd.yaml Verifying diff on 0000_10_config-operator_01_oauths.crd.yaml Verifying diff on 0000_10_config-operator_01_projects.crd.yaml Verifying diff on 0000_10_config-operator_01_schedulers-CustomNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_schedulers-Default.crd.yaml Verifying diff on 0000_10_config-operator_01_schedulers-DevPreviewNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_schedulers-TechPreviewNoUpgrade.crd.yaml Verifying diff on 0000_03_config-operator_01_clusterresourcequotas.crd.yaml Verifying diff on 0000_03_config-operator_01_securitycontextconstraints.crd.yaml Verifying diff on 0000_03_config-operator_02_rangeallocations.crd.yaml Verifying diff on 0000_03_config-operator_01_rolebindingrestrictions.crd.yaml Verifying diff on 0000_10_config-operator_01_imagecontentsourcepolicies.crd.yaml Verifying diff on 0000_10_config-operator_01_configs.crd.yaml Found untracked file 0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml in payload CRD manifests. Please add the file to crd_globs in hack/update-payload-crds.sh.
I follow the same process of copying
cp payload-manifests/crds/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml config/v1/zz_generated.crd-manifests
Then make verify passes:
misalunk@misalunk-mac api % ./hack/verify-payload-crds.sh Verifying diff on 0000_03_config-operator_01_proxies.crd.yaml Verifying diff on 0000_10_config-operator_01_apiservers.crd.yaml Verifying diff on 0000_10_config-operator_01_authentications-Hypershift.crd.yaml Verifying diff on 0000_10_config-operator_01_authentications-SelfManagedHA-CustomNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_authentications-SelfManagedHA-Default.crd.yaml Verifying diff on 0000_10_config-operator_01_authentications-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_authentications-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_consoles.crd.yaml Verifying diff on 0000_10_config-operator_01_dnses.crd.yaml Verifying diff on 0000_10_config-operator_01_featuregates.crd.yaml Verifying diff on 0000_10_config-operator_01_imagecontentpolicies.crd.yaml Verifying diff on 0000_10_config-operator_01_imagedigestmirrorsets.crd.yaml Verifying diff on 0000_10_config-operator_01_images.crd.yaml Verifying diff on 0000_10_config-operator_01_imagetagmirrorsets.crd.yaml Verifying diff on 0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_infrastructures-Default.crd.yaml Verifying diff on 0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_ingresses-CustomNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_ingresses-Default.crd.yaml Verifying diff on 0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_ingresses.crd.yaml Verifying diff on 0000_10_config-operator_01_networks-CustomNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_networks-Default.crd.yaml Verifying diff on 0000_10_config-operator_01_networks-DevPreviewNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_networks-TechPreviewNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_nodes.crd.yaml Verifying diff on 0000_10_config-operator_01_oauths.crd.yaml Verifying diff on 0000_10_config-operator_01_projects.crd.yaml Verifying diff on 0000_10_config-operator_01_schedulers-CustomNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_schedulers-Default.crd.yaml Verifying diff on 0000_10_config-operator_01_schedulers-DevPreviewNoUpgrade.crd.yaml Verifying diff on 0000_10_config-operator_01_schedulers-TechPreviewNoUpgrade.crd.yaml Verifying diff on 0000_03_config-operator_01_clusterresourcequotas.crd.yaml Verifying diff on 0000_03_config-operator_01_securitycontextconstraints.crd.yaml Verifying diff on 0000_03_config-operator_02_rangeallocations.crd.yaml Verifying diff on 0000_03_config-operator_01_rolebindingrestrictions.crd.yaml Verifying diff on 0000_10_config-operator_01_imagecontentsourcepolicies.crd.yaml Verifying diff on 0000_10_config-operator_01_configs.crd.yaml
However after running make update these files again go away from the config/v1/zz_generated.crd-manifests/
Can you please help on this ?
@deads2k I am still getting this issue after running make verify as mentioned in https://github.com/openshift/api/pull/1826#issuecomment-2069725575 :
Found untracked file 0000_10_config-operator_01_ingresses.crd.yaml in payload CRD manifests. Please add the file to crd_globs in hack/update-payload-crds.sh. make: *** [verify-scripts] Error 1 misalunk@misalunk-mac api %
@miheer: 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-ovn-hypershift | 24253ce8e0ca5da2161ba0ea4abc7f5659b8ae4c | link | true | /test e2e-aws-ovn-hypershift |
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/test-infra repository. I understand the commands that are listed here.
After rebasing with master I am getting this issue after while running make verify as ./hack/verify-crd-schema-checker.sh fails with the following: @deads2k @JoelSpeed can you please help on this ?
I0523 16:07:43.536941 61286 generator.go:126] No CRD manifests found for user.openshift.io/v1 F0523 16:07:43.536960 61286 root.go:64] Error running codegen: error running generator schemacheck on group config.openshift.io: could not run schemacheck generator for group/version config.openshift.io/v1: error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-CustomNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.status.componentRoutes[].relatedObjects must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-CustomNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.status.componentRoutes[].consumingUsers must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-CustomNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.status.componentRoutes[].currentHostnames must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-CustomNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.spec.requiredHSTSPolicies must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-CustomNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.spec.requiredHSTSPolicies[].domainPatterns must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-CustomNoUpgrade.crd.yaml: NoMaps: crd/ingresses.config.openshift.io version/v1 field/^.spec.requiredHSTSPolicies[].namespaceSelector.matchLabels may not be a map error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-Default.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.spec.requiredHSTSPolicies must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-Default.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.spec.requiredHSTSPolicies[].domainPatterns must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-Default.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.status.componentRoutes[].consumingUsers must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-Default.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.status.componentRoutes[].currentHostnames must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-Default.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.status.componentRoutes[].relatedObjects must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-Default.crd.yaml: NoMaps: crd/ingresses.config.openshift.io version/v1 field/^.spec.requiredHSTSPolicies[].namespaceSelector.matchLabels may not be a map error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.spec.requiredHSTSPolicies must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.spec.requiredHSTSPolicies[].domainPatterns must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.status.componentRoutes[].consumingUsers must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.status.componentRoutes[].currentHostnames must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.status.componentRoutes[].relatedObjects must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-DevPreviewNoUpgrade.crd.yaml: NoMaps: crd/ingresses.config.openshift.io version/v1 field/^.spec.requiredHSTSPolicies[].namespaceSelector.matchLabels may not be a map error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.spec.requiredHSTSPolicies must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.spec.requiredHSTSPolicies[].domainPatterns must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.status.componentRoutes[].relatedObjects must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.status.componentRoutes[].consumingUsers must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.status.componentRoutes[].currentHostnames must set x-kubernetes-list-type error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml: NoMaps: crd/ingresses.config.openshift.io version/v1 field/^.spec.requiredHSTSPolicies[].namespaceSelector.matchLabels may not be a map error in config/v1/zz_generated.featuregated-crd-manifests/ingresses.config.openshift.io/SetEIPForNLBIngressController.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.spec.requiredHSTSPolicies must set x-kubernetes-list-type error in config/v1/zz_generated.featuregated-crd-manifests/ingresses.config.openshift.io/SetEIPForNLBIngressController.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.spec.requiredHSTSPolicies[].domainPatterns must set x-kubernetes-list-type error in config/v1/zz_generated.featuregated-crd-manifests/ingresses.config.openshift.io/SetEIPForNLBIngressController.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.status.componentRoutes[].consumingUsers must set x-kubernetes-list-type error in config/v1/zz_generated.featuregated-crd-manifests/ingresses.config.openshift.io/SetEIPForNLBIngressController.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.status.componentRoutes[].currentHostnames must set x-kubernetes-list-type error in config/v1/zz_generated.featuregated-crd-manifests/ingresses.config.openshift.io/SetEIPForNLBIngressController.yaml: ListsMustHaveSSATags: crd/ingresses.config.openshift.io version/v1 field/^.status.componentRoutes[].relatedObjects must set x-kubernetes-list-type error in config/v1/zz_generated.featuregated-crd-manifests/ingresses.config.openshift.io/SetEIPForNLBIngressController.yaml: NoMaps: crd/ingresses.config.openshift.io version/v1 field/^.spec.requiredHSTSPolicies[*].namespaceSelector.matchLabels may not be a map
@miheer Most of the failures are because you are adding a gated field to an existing struct, thus splitting a single CRD into multiple, this is normal and expected, violations are present already. However, we do ask when folks hit this, to resolve any SSA tags for lists that are missing, you'll see a few of these in the result there.
Your options are // +listType=atomic or // +listType=map and then // +listMapKey=... to identify the key. The default is atomic, and the map type should only be used on lists of complex structures where there is a clear and unique identifier, or identifier combination from multiple fields.
Please can you review the existing SSA tag failures and see if you can resolve those at least
@JoelSpeed I am getting the following error even when I have mentioned maxItems for EIPAllocations.
Can you please help on this ?
[FAILED] [0.014 seconds] [config.openshift.io/v1, Resource=ingresses][ClusterProfiles=Hypershift,SelfManagedHA][FeatureSet="TechPreviewNoUpgrade"][FeatureGate=SetEIPForNLBIngressController][File=0000_10_config-operator_01_ingresses-TechPreviewNoUpgrade.crd.yaml] Ingress [BeforeEach] On Create Should not allow to set NLB parameters when LBType is Classic. [BeforeEach] /Users/misalunk/go/src/github.com/openshift/api/tests/generator.go:109 [It] /Users/misalunk/go/src/github.com/openshift/api/tests/generator.go:189
[FAILED] Unexpected error: <*fmt.wrapError | 0xc00130e580>: unable to create CRD instances: unable to create CRD "ingresses.config.openshift.io": CustomResourceDefinition.apiextensions.k8s.io "ingresses.config.openshift.io" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[loadBalancer].properties[platform].properties[aws].properties[networkLoadBalancer].properties[eipAllocations].x-kubernetes-validations[0].rule: Forbidden: estimated rule cost exceeds budget by factor of 3.1x (try simplifying the rule, or adding maxItems, maxProperties, and maxLength where arrays, maps, and strings are declared) { msg: "unable to create CRD instances: unable to create CRD "ingresses.config.openshift.io": CustomResourceDefinition.apiextensions.k8s.io "ingresses.config.openshift.io" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[loadBalancer].properties[platform].properties[aws].properties[networkLoadBalancer].properties[eipAllocations].x-kubernetes-validations[0].rule: Forbidden: estimated rule cost exceeds budget by factor of 3.1x (try simplifying the rule, or adding maxItems, maxProperties, and maxLength where arrays, maps, and strings are declared)", err: <*fmt.wrapError | 0xc00130e4e0>{ msg: "unable to create CRD "ingresses.config.openshift.io": CustomResourceDefinition.apiextensions.k8s.io "ingresses.config.openshift.io" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[loadBalancer].properties[platform].properties[aws].properties[networkLoadBalancer].properties[eipAllocations].x-kubernetes-validations[0].rule: Forbidden: estimated rule cost exceeds budget by factor of 3.1x (try simplifying the rule, or adding maxItems, maxProperties, and maxLength where arrays, maps, and strings are declared)", err: <*errors.StatusError | 0xc005b214a0>{ ErrStatus: { TypeMeta: {Kind: "", APIVersion: ""}, ListMeta: { SelfLink: "", ResourceVersion: "", Continue: "", RemainingItemCount: nil, }, Status: "Failure", Message: "CustomResourceDefinition.apiextensions.k8s.io "ingresses.config.openshift.io" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[loadBalancer].properties[platform].properties[aws].properties[networkLoadBalancer].properties[eipAllocations].x-kubernetes-validations[0].rule: Forbidden: estimated rule cost exceeds budget by factor of 3.1x (try simplifying the rule, or adding maxItems, maxProperties, and maxLength where arrays, maps, and strings are declared)", Reason: "Invalid", Details: { Name: "ingresses.config.openshift.io", Group: "apiextensions.k8s.io", Kind: "CustomResourceDefinition", UID: "", Causes: [ { Type: "FieldValueForbidden", Message: "Forbidden: estimated rule cost exceeds budget by factor of 3.1x (try simplifying the rule, or adding maxItems, maxProperties, and maxLength where arrays, maps, and strings are declared)", Field: "spec.validation.openAPIV3Schema.properties[spec].properties[loadBalancer].properties[platform].properties[aws].properties[networkLoadBalancer].properties[eipAllocations].x-kubernetes-validations[0].rule", }, ], RetryAfterSeconds: 0, }, Code: 422, }, }, }, } occurred In [BeforeEach] at: /Users/misalunk/go/src/github.com/openshift/api/tests/generator.go:119 @ 06/05/24 10:00:03.395
There were additional failures detected. To view them in detail run ginkgo -vv
@miheer Most of the failures are because you are adding a gated field to an existing struct, thus splitting a single CRD into multiple, this is normal and expected, violations are present already. However, we do ask when folks hit this, to resolve any SSA tags for lists that are missing, you'll see a few of these in the result there.
Your options are
// +listType=atomicor// +listType=mapand then// +listMapKey=...to identify the key. The default is atomic, and the map type should only be used on lists of complex structures where there is a clear and unique identifier, or identifier combination from multiple fields.Please can you review the existing SSA tag failures and see if you can resolve those at least
@JoelSpeed The errors don't seem to be related to my API. So, do you want me to make changes to different APIs ?
@miheer I created https://issues.redhat.com/browse/OCPBUGS-34906 to resolve the unrelated-to-this-PR verify-crd-schema failures. I think @JoelSpeed is amenable overriding since we are tracking this work elsewhere, but correct me if I'm wrong @JoelSpeed.
The commit message needs to be fixed to remove a reference to scaling IngressControllers (copypasta?), to reflect changes to API field names, and to conform to formatting conventions:
Add configuration for AWS Elastic IPs (EIPs)
Add fields in the cluster Ingress config API and IngressController API
to specify Elastic IPs (EIPs) for ELBs associated with
IngressControllers.
The Ingress Operator uses these fields to set the
service.beta.kubernetes.io/aws-load-balancer-eip-allocations service
annotation. The eipAllocations field in the cluster config specifies
EIPs for the default IngressController. The eipAllocations field in the
IngressController object takes precedence over the cluster config and
can be used for non-default IngressControllers.
This commit implements NE-1516.
https://issues.redhat.com/browse/NE-1516
* config/v1/types_ingress.go: Add an API field eipAllocations in the
Ingress config CRD. Its value is set by the installer from
install-config.yaml.
* operator/v1/types_ingress.go: Add an API field eipAllocations in the
IngressController CRD.
There are quite a few outstanding comments on this PR from previous reviews, as well as some that are fixed but not resolved, can we try to mark them as resolved if they are completed.
changes
There are quite a few outstanding comments on this PR from previous reviews, as well as some that are fixed but not resolved, can we try to mark them as resolved if they are completed.
OK now I have tried to reply to each comment saying "done" which are fixed. For some comments I have asked questions and replied back. Please do let me know if still something is missed out.
@JoelSpeed @Miciah @gcs278 PTAL
@gcs278 @JoelSpeed PTAL I have also removed the config/v1 part as the installer part of this epic is not doable this time so not including the config/v1 this time.
@miheer: This pull request references NE-1516 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:
Add a field in the IngressController API to specify Elastic IPs (EIPs) for ELBs associated with IngressControllers.
The Ingress Operator uses this field to set the service.beta.kubernetes.io/aws-load-balancer-eip-allocations service annotation.
This commit implements NE-1516.
https://issues.redhat.com/browse/NE-1516
- operator/v1/types_ingress.go: Add an API field eipAllocations in the IngressController CRD.
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.
@miheer: This pull request references NE-1516 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:
Add a field in the IngressController API to specify Elastic IPs (EIPs) for ELBs associated with IngressControllers.
The Ingress Operator uses this field to set the service.beta.kubernetes.io/aws-load-balancer-eip-allocations service annotation.
This commit implements NE-1516.
https://issues.redhat.com/browse/NE-1516
- operator/v1/types_ingress.go: Add an API field eipAllocations in the IngressController CRD.
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.
@miheer You have a verify failure diff: /go/src/github.com/openshift/api/payload-manifests/crds/0000_10_config-operator_01_ingresses.crd.yaml: No such file or directory
I think we are straddling the feature gate CRD split for IngressConfig. Now that you reverted, we shouldn't splitting the IngressConfig CRD files anymore, you will probably need to manually delete payload-manifests/crds/0000_10_config-operator_01_ingresses-*.crd.yaml and run make update again and figure out how to produce 0000_10_config-operator_01_ingresses.crd.yaml again.
/lgtm
Thanks for getting the godoc updated, all looks good now
/override ci/prow/verify-crd-schema
Existing failures for NoMaps, nothing we can do to resolve that now
@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/verify-crd-schema
In response to this:
/lgtm
Thanks for getting the godoc updated, all looks good now
/override ci/prow/verify-crd-schema
Existing failures for NoMaps, nothing we can do to resolve that now
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.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: gcs278, JoelSpeed, miheer
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [JoelSpeed]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
/retest-required
Remaining retests: 0 against base HEAD 442f06d7e03bc1ffb68816dfcd814409cd0f7e28 and 2 for PR HEAD caab6d62b47ef539c2cc52dd675e4828a8379231 in total
@miheer: all tests passed!
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.
[ART PR BUILD NOTIFIER]
Distgit: ose-cluster-config-api This PR has been included in build ose-cluster-config-api-container-v4.17.0-202407242114.p0.g84047ef.assembly.stream.el9. All builds following this will include this PR.
