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

NE-1808: Bump controller to v2.8.2 #139

Merged

Conversation

alebedev87
Copy link
Contributor

@alebedev87 alebedev87 commented Dec 9, 2024

  • Updated controller image to the latest version from downstream.
  • Re-generated operator bundle using make bundle, which also updated controller CRDs.
  • Synced IAM policy with the latest downstream version.
  • Re-generated managed IAM policy and credentials request using make generate.
  • Updated managed controller RBAC to include permissions for leases.
    • Note: Controller-runtime no longer supports ConfigMaps for leader election.

Integrates openshift/aws-load-balancer-controller#23 into the operator.

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

openshift-ci-robot commented Dec 9, 2024

@alebedev87: This pull request references NE-1807 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:

  • Updated controller image to the latest version from downstream.
  • Re-generated operator bundle using make bundle, which also updated controller CRDs.
  • Synced IAM policy with the latest downstream version.
  • Re-generated managed IAM policy and credentials request using make generate.
  • Updated managed controller RBAC to include permissions for leases.
  • Note: Controller-runtime no longer supports ConfigMaps for leader election.

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-ci openshift-ci bot requested review from frobware and miheer December 9, 2024 15:49
@openshift-ci-robot
Copy link

openshift-ci-robot commented Dec 9, 2024

@alebedev87: This pull request references NE-1807 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:

  • Updated controller image to the latest version from downstream.
  • Re-generated operator bundle using make bundle, which also updated controller CRDs.
  • Synced IAM policy with the latest downstream version.
  • Re-generated managed IAM policy and credentials request using make generate.
  • Updated managed controller RBAC to include permissions for leases.
  • Note: Controller-runtime no longer supports ConfigMaps for leader election.

Integrates openshift/aws-load-balancer-controller#23 into the operator.

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.

@alebedev87 alebedev87 changed the title NE-1807: Bump controller to v2.8.2 NE-1808: Bump controller to v2.8.2 Dec 9, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Dec 9, 2024

@alebedev87: This pull request references NE-1808 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:

  • Updated controller image to the latest version from downstream.
  • Re-generated operator bundle using make bundle, which also updated controller CRDs.
  • Synced IAM policy with the latest downstream version.
  • Re-generated managed IAM policy and credentials request using make generate.
  • Updated managed controller RBAC to include permissions for leases.
  • Note: Controller-runtime no longer supports ConfigMaps for leader election.

Integrates openshift/aws-load-balancer-controller#23 into the operator.

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.

@alebedev87 alebedev87 force-pushed the integrate-rebase-2-8-2 branch from 82af5cc to c801a1e Compare December 10, 2024 09:58
- Updated controller image to the latest version from downstream.
- Re-generated operator bundle using `make bundle`, which also updated controller CRDs.
- Synced IAM policy with the latest downstream version.
- Re-generated managed IAM policy and credentials request using `make generate`.
- Updated managed controller RBAC to include permissions for leases.
  - Note: Controller-runtime no longer supports ConfigMaps for leader election.
@alebedev87 alebedev87 force-pushed the integrate-rebase-2-8-2 branch from c801a1e to 0089375 Compare December 10, 2024 09:59
@alebedev87
Copy link
Contributor Author

/test e2e-aws-rosa-operator

WAF ACL creation may conflict with other PRs.
openshift/release#59728 addresses this.

@alebedev87
Copy link
Contributor Author

/test e2e-aws-proxy-operator

@alebedev87
Copy link
Contributor Author

/test e2e-aws-rosa-operator

@alebedev87
Copy link
Contributor Author

alebedev87 commented Dec 11, 2024

The e2e test passed but

Process did not finish before 4h0m0s timeout

Addressed in openshift/release#59745.

/test e2e-aws-rosa-operator

@candita
Copy link

candita commented Dec 11, 2024

/assign @gcs278

@alebedev87
Copy link
Contributor Author

/retest

@@ -38,7 +38,8 @@
"elasticloadbalancing:DescribeTargetGroups",
"elasticloadbalancing:DescribeTargetGroupAttributes",
"elasticloadbalancing:DescribeTargetHealth",
"elasticloadbalancing:DescribeTags"
"elasticloadbalancing:DescribeTags",
"elasticloadbalancing:DescribeTrustStores"
Copy link

@gcs278 gcs278 Dec 13, 2024

Choose a reason for hiding this comment

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

Is updating the iam-policy automated at all, or do you just have to manually look at https://github.com/openshift/aws-load-balancer-controller/blob/d0c13bf1576965a3b65fc09ebce94ed9f86833a2/docs/install/iam_policy.json to see if anything changed and manually sync it?

Edit: I commented on the wrong file, I know there's iamctl to sync iam-policy within the ALBO repo, but just curious if the upstream change is manually synced to this repo, and if so, is that something that we could fix in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now, the process is manual. Initially I planned to automate it in this PR and even created a dedicated hack file for that purpose. However, I noticed that the semantic difference can sometimes be much smaller than the byte-by-byte difference because certain statements might be reshuffled upstream.

In this particular case, the semantic change was limited to adding the elasticloadbalancing:DescribeTrustStores action for the mTLS support (which we don't support yet).

I couldn’t find a straightforward way to sort the upstream policy that would minimize the diff while avoiding the risk of losing statements. As a result, I decided to keep the process manual so that multiple people can validate the changes.

Copy link

Choose a reason for hiding this comment

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

Makes sense. If someone in the future missed a iam-policy update on a rebase, would you expect it to get caught by E2E tests? Or is it a solid "maybe"?

Copy link
Contributor Author

@alebedev87 alebedev87 Dec 16, 2024

Choose a reason for hiding this comment

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

Not as solid as I would like it to be. The e2e tests cover only the scenarios described in the docs. If IAM policy changes go beyond this - we may miss them.

@gcs278
Copy link

gcs278 commented Dec 13, 2024

Do you need to bump

sigs.k8s.io/aws-load-balancer-controller v0.0.0-20220923211742-8d282339857c

in the go.mod file? I see it's only used for operator_test.go, so it might not matter, but maybe it's good to do it for completeness, as it appears the schema has changed on this bump.

@alebedev87
Copy link
Contributor Author

alebedev87 commented Dec 16, 2024

Do you need to bump sigs.k8s.io/aws-load-balancer-controller in the go.mod file? I see it's only used for operator_test.go, so it might not matter, but maybe it's good to do it for completeness, as it appears the schema has changed on this bump.

Right, I do. Let me do it in #143 because it needs k8s.io 0.30.z. Upd: done.

@alebedev87
Copy link
Contributor Author

/retest

@gcs278
Copy link

gcs278 commented Dec 16, 2024

Thanks!
/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 16, 2024
Copy link
Contributor

openshift-ci bot commented Dec 16, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gcs278

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:

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 Dec 16, 2024
@alebedev87
Copy link
Contributor Author

/label px-approved

The docs are enough for this feature.

@openshift-ci openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Dec 17, 2024
@ahardin-rh
Copy link

/label docs-approved

@openshift-ci openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Dec 17, 2024
@ShudiLi
Copy link
Member

ShudiLi commented Dec 18, 2024

tested it with 4.18.0-0.ci.test-2024-12-18-012548-ci-ln-h9xtjxb-latest

1.
% oc get clusterversion
NAME      VERSION                                                   AVAILABLE   PROGRESSING   SINCE   STATUS
version   4.18.0-0.ci.test-2024-12-18-012548-ci-ln-h9xtjxb-latest   True        False         37m     Cluster version is 4.18.0-0.ci.test-2024-12-18-012548-ci-ln-h9xtjxb-latest

2.
% oc -n aws-load-balancer-operator  get pods                                                                                                                 
NAME                                                              READY   STATUS      RESTARTS   AGE
35b5a855d06d8296bb3b140bd95fc6eb70ff19c94d535f5b2a707bea99c8865   1/1     Running     0          31m
aws-load-balancer-controller-cluster-7c5c987cfd-qwrb2             1/1     Running     0          22m
aws-load-balancer-operator-controller-manager-b7fdf64bc-2mw4w     2/2     Running     0          31m
da32229ded5cc96453559786df040a1e59fbaa3ba6dbf73ddb11aa3117zxnz2   0/1     Completed   0          31m

3.
% oc -n aws-load-balancer-operator  get pod aws-load-balancer-operator-controller-manager-b7fdf64bc-2mw4w -oyaml | grep -i quay.io/aws-load-balancer-operator
      value: quay.io/aws-load-balancer-operator/aws-load-balancer-controller@sha256:75c5e5c1c650b27c273875d6e9497ede335c0ded6719064b0db63a6da8369937

4.
% oc get CustomResourceDefinition ingressclassparams.elbv2.k8s.aws -oyaml
% oc get CustomResourceDefinition ingressclassparams.elbv2.k8s.aws -oyaml | grep "controller-gen.kubebuilder.io/version"
    controller-gen.kubebuilder.io/version: v0.14.0
% oc get CustomResourceDefinition ingressclassparams.elbv2.k8s.aws -oyaml | grep "APIVersion defines the versioned schema of this representation of an object" 
              APIVersion defines the versioned schema of this representation of an object.
% oc get CustomResourceDefinition ingressclassparams.elbv2.k8s.aws -oyaml | grep "Kind is a string value representing the REST resource this object represents"
              Kind is a string value representing the REST resource this object represents.

5.
% oc -n test get ingress
NAME           CLASS   HOSTS                   ADDRESS                                                               PORTS   AGE
ingress-test   alb     www.user2-example.com   k8s-test-ingresst-4efc854f6c-1287644373.us-east-1.elb.amazonaws.com   80      3m38s

6.
% curl http://k8s-test-ingresst-4efc854f6c-1287644373.us-east-1.elb.amazonaws.com -H "host: www.user2-example.com"
Hello-OpenShift web-server-1 http-8080

@ShudiLi
Copy link
Member

ShudiLi commented Dec 18, 2024

/label qe-approved
thanks

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Dec 18, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Dec 18, 2024

@alebedev87: This pull request references NE-1808 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:

  • Updated controller image to the latest version from downstream.
  • Re-generated operator bundle using make bundle, which also updated controller CRDs.
  • Synced IAM policy with the latest downstream version.
  • Re-generated managed IAM policy and credentials request using make generate.
  • Updated managed controller RBAC to include permissions for leases.
  • Note: Controller-runtime no longer supports ConfigMaps for leader election.

Integrates openshift/aws-load-balancer-controller#23 into the operator.

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

/retest-required

Remaining retests: 0 against base HEAD 1ac0bcf and 2 for PR HEAD 0089375 in total

@ShudiLi
Copy link
Member

ShudiLi commented Dec 18, 2024

/retest-required

Copy link
Contributor

openshift-ci bot commented Dec 18, 2024

@alebedev87: 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.

@openshift-merge-bot openshift-merge-bot bot merged commit 82a317a into openshift:main Dec 18, 2024
10 checks passed
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. docs-approved Signifies that Docs has signed off on this PR 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. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants