Skip to content

Conversation

@patrickdillon
Copy link
Contributor

4.12 backport of #1529

Adds a field to the DNS config to hold a role to assume when performing cross-account installs in an AWS shared VPC environment.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 8, 2023
@openshift-ci-robot
Copy link

openshift-ci-robot commented Aug 8, 2023

@patrickdillon: This pull request references OCPSTRAT-660 which is a valid jira issue.

Details

In response to this:

4.12 backport of #1529

Adds a field to the DNS config to hold a role to assume when performing cross-account installs in an AWS shared VPC environment.

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.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 8, 2023

Hello @patrickdillon! 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 openshift-ci bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 8, 2023
@openshift-ci openshift-ci bot requested review from adambkaplan and bparees August 8, 2023 18:51
@patrickdillon
Copy link
Contributor Author

cc @gcs278

@gcs278
Copy link
Contributor

gcs278 commented Aug 8, 2023

@patrickdillon thanks - looks like there is an verify failure on verify-update-codegen-crds. I'll vendor it in a NE PR once that is worked out.

@patrickdillon
Copy link
Contributor Author

@patrickdillon thanks - looks like there is an verify failure on verify-update-codegen-crds. I'll vendor it in a NE PR once that is worked out.

hah! yeah I was waiting to dm you in slack until I fixed this. forgot I cc'd you in gh.

Hopefully this push fixed it.

@patrickdillon
Copy link
Contributor Author

patrickdillon commented Aug 10, 2023

When I run make update locally I'm getting some extra CRDs, which, if included, break the verify-update-codegen-crds test. Two examples:

$ git diff
diff --git a/example/v1alpha1/0000_50_notstabletype-techpreview.crd.yaml b/example/v1alpha1/0000_50_notstabletype-techpreview.crd.yaml
index 263f945f..f5c352b6 100644
--- a/example/v1alpha1/0000_50_notstabletype-techpreview.crd.yaml
+++ b/example/v1alpha1/0000_50_notstabletype-techpreview.crd.yaml
@@ -64,19 +64,30 @@ spec:
                       message:
                         description: message is a human readable message indicating details about the transition. This may be an empty string.
                         type: string
+                        maxLength: 32768
                       observedGeneration:
                         description: observedGeneration represents the .metadata.generation that the condition was set based upon. For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date with respect to the current state of the instance.
                         type: integer
                         format: int64
+                        minimum: 0
                       reason:
                         description: reason contains a programmatic identifier indicating the reason for the condition's last transition. Producers of specific condition types may define expected values and meanings for this field, and whether the values are considered a guaranteed API. The value should be a CamelCase string. This field may not be empty.
                         type: string
+                        maxLength: 1024
+                        minLength: 1
+                        pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
                       status:
                         description: status of the condition, one of True, False, Unknown.
                         type: string
+                        enum:
+                          - "True"
+                          - "False"
+                          - Unknown
                       type:
                         description: type of condition in CamelCase or in foo.example.com/CamelCase. --- Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be useful (see .node.status.conditions), the ability to deconflict is important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
                         type: string
+                        maxLength: 316
+                        pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
                   x-kubernetes-list-map-keys:
                     - type
                   x-kubernetes-list-type: map
diff --git a/machine/v1beta1/0000_10_machine.crd.yaml b/machine/v1beta1/0000_10_machine.crd.yaml
index fc67ec1e..4b968e51 100644
--- a/machine/v1beta1/0000_10_machine.crd.yaml
+++ b/machine/v1beta1/0000_10_machine.crd.yaml
@@ -175,6 +175,7 @@ spec:
                           uid:
                             description: 'UID of the referent. More info: http://kubernetes.io/docs/user-guide/identifiers#uids'
                             type: string
+                        x-kubernetes-map-type: atomic
                 providerID:
                   description: ProviderID is the identification ID of the machine provided by the provider. This field must match the provider ID as seen on the node object corresponding to this machine. This field is required by higher level consumers of cluster-api. Example use case is cluster autoscaler with cluster-api as provider. Clean-up logic in the autoscaler compares machines to nodes to find out machines at provider which could not get registered as Kubernetes nodes. With cluster-api as a generic out-of-tree provider for autoscaler, this field is required by autoscaler to be able to have a provider view of the list of machines. Another list of nodes is queried from the k8s apiserver and then a comparison is done to find out unregistered machines and are marked for delete. This field will be set by the actuators and consumed by higher level entities like autoscaler that will be interfacing with cluster-api as generic provider.
                   type: string
	modified:   machine/v1beta1/0000_10_machinehealthcheck.yaml
	modified:   machine/v1beta1/0000_10_machineset.crd.yaml

have the same diff as the last example, adding: + x-kubernetes-map-type: atomic

@patrickdillon
Copy link
Contributor Author

/hold
While we investigate #1551 (comment)

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 10, 2023
@patrickdillon
Copy link
Contributor Author

/hold cancel
Joel pointed me to make -C tools clean codegen which resolved any doubts about this issue. Ready to merge.

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 17, 2023
@deads2k
Copy link
Contributor

deads2k commented Aug 17, 2023

/approve
/lgtm

@deads2k deads2k added the backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. label Aug 17, 2023
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 17, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 17, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, patrickdillon

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 17, 2023
@sdodson sdodson added cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels Aug 17, 2023
@sdodson
Copy link
Member

sdodson commented Aug 17, 2023

Adding jira/valid-bug for this approved feature backport.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 17, 2023

@patrickdillon: all tests passed!

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants