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

Update k8s dependencies #67

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

Neo2308
Copy link

@Neo2308 Neo2308 commented Apr 26, 2024

  • Updated k8s dependencies to v0.29.3
  • Updated operator-framework/operator-registry to v1.39.0
  • Updated kubebuilder to v3.14.2
  • Updated k8s version in Makefile

Relates to: operator-framework/operator-sdk#6651

@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 Apr 26, 2024
@Neo2308 Neo2308 changed the title {WIP} Update k8s dependencies Update k8s dependencies May 1, 2024
@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 May 1, 2024
@Neo2308
Copy link
Author

Neo2308 commented May 1, 2024

@rashmigottipati could you help review this PR?

go.mod Outdated Show resolved Hide resolved
Neo2308 added a commit to Neo2308/operator-framework-operator-sdk that referenced this pull request May 1, 2024
* Bump k8s dependencies to 0.29.3
* Bump other operator-framework lib versions to support k8s 0.29.x
* Bump ginkgo/v2 to v2.17.2

TODO:
* Update operator-framework/ansible-operator-plugins after operator-framework/ansible-operator-plugins#67 is merged
* Add changelog
* Regenerate testdata

Signed-off-by: Neo2308 <pradha.krishna.cse17@itbhu.ac.in>
Copy link
Collaborator

@everettraven everettraven left a comment

Choose a reason for hiding this comment

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

These changes look good, but I think we also need to make sure we update the python packages provided in the base image here:

@Neo2308 Neo2308 requested a review from everettraven May 3, 2024 06:24
pkg/plugins/util/cleanup.go Outdated Show resolved Hide resolved
@Neo2308
Copy link
Author

Neo2308 commented May 7, 2024

@everettraven could you review this PR again?

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 11, 2024
@Neo2308 Neo2308 force-pushed the feature/main/bump-k8s-0.29.3 branch from 59295f4 to c1fc012 Compare May 15, 2024 06:53
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 15, 2024
@Neo2308
Copy link
Author

Neo2308 commented May 15, 2024

@everettraven could you review this PR since its blocking operator-framework/operator-sdk#6736?

@Neo2308
Copy link
Author

Neo2308 commented May 21, 2024

@everettraven / @rashmigottipati can you help review this PR? This blocking work on the 1.29 bump

go.mod Outdated Show resolved Hide resolved
Copy link
Collaborator

@everettraven everettraven left a comment

Choose a reason for hiding this comment

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

Changes LGTM aside from the comment that @acornett21 left

@Neo2308
Copy link
Author

Neo2308 commented May 22, 2024

Made the specified changes. @everettraven, @acornett21 could you review again?

Neo2308 added a commit to Neo2308/operator-framework-operator-sdk that referenced this pull request May 22, 2024
* Bump k8s dependencies to 0.29.3
* Bump other operator-framework lib versions to support k8s 0.29.x
* Bump ginkgo/v2 to v2.17.2

TODO:
* Update operator-framework/ansible-operator-plugins after operator-framework/ansible-operator-plugins#67 is merged
* Add changelog
* Regenerate testdata

Signed-off-by: Neo2308 <pradha.krishna.cse17@itbhu.ac.in>
@Neo2308
Copy link
Author

Neo2308 commented May 27, 2024

The e2e-molecule test failure seems to be due to an environment issue, I don't seem to have the permissions to retrigger it.
@joelanford could you help retrigger the checks?

@acornett21
Copy link
Contributor

@Neo2308 I think I've re-ran this test 2 or 3 times, and it's still failing.

@Neo2308
Copy link
Author

Neo2308 commented Jun 7, 2024

@acornett21 could you help fix the build failure?

@acornett21
Copy link
Contributor

@Neo2308 It looks like this is a python dep issue, I took this code as a base and raised #80 with a few changes and all seems well. I reached out on k8s slack to try to get some time to talk about this PR and #80 and #79.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 14, 2024
* Updated k8s dependencies to v0.29.3
* Updated operator-framework/operator-registry to v1.39.0
* Updated kubebuilder to v3.14.1
* Updated k8s version in Makefile

TODO:
Update operator-framework/operator-lib after operator-framework/operator-lib#167 is merged
* Updated operator-framework/operator-lib to v1.39.0
Use NewDynamicRESTMapper instead of NewDiscoveryRESTMapper (Ref: kubernetes-sigs/controller-runtime#2611)
* Use operator-lib v0.13.0 instead of commit.
* Regenerated testdata.
@Neo2308 Neo2308 force-pushed the feature/main/bump-k8s-0.29.3 branch from d85c877 to 9c82fc3 Compare June 18, 2024 06:06
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 18, 2024
@Neo2308
Copy link
Author

Neo2308 commented Jun 18, 2024

Rebased the PR onto latest master. @acornett21 could you approve the workflow runs?

@Neo2308
Copy link
Author

Neo2308 commented Jun 20, 2024

The sanity tests are passing locally now, @acornett21 / @everettraven could you run approve the workflow runs?

@acornett21
Copy link
Contributor

acornett21 commented Jun 25, 2024

@Neo2308 It doesn't appear that you generated the lock file correctly, you need to follow the instructions in the images/Readme file. But even if that is generated correctly there are still other issues, since version of requests needed to get past the CVE does not support the use case (protocol) we are using.

Azure/azure-iot-sdk-python#1182

Controller Logs

{"level":"error","ts":"2024-06-25T14:43:26Z","logger":"runner","msg":"Traceback (most recent call last):\n  File \"/usr/local/lib/python3.9/site-packages/requests/adapters.py\", line 633, in send\n    conn = self.get_connection_with_tls_context(\n  File \"/usr/local/lib/python3.9/site-packages/requests/adapters.py\", line 489, in get_connection_with_tls_context\n    conn = self.poolmanager.connection_from_host(\n  File \"/usr/local/lib/python3.9/site-packages/urllib3/poolmanager.py\", line 246, in connection_from_host\n    return self.connection_from_context(request_context)\n  File \"/usr/local/lib/python3.9/site-packages/urllib3/poolmanager.py\", line 258, in connection_from_context\n    raise URLSchemeUnknown(scheme)\nurllib3.exceptions.URLSchemeUnknown: Not supported URL scheme http+unix\n\nDuring handling of the above exception, another exception occurred:\n\nTraceback (most recent call last):\n  File \"/usr/local/lib/python3.9/site-packages/ansible_runner/__main__.py\", line 874, in main\n    res = run(**run_options)\n  File \"/usr/local/lib/python3.9/site-packages/ansible_runner/interface.py\", line 210, in run\n    r.run()\n  File \"/usr/local/lib/python3.9/site-packages/ansible_runner/runner.py\", line 118, in run\n    self.status_callback('starting')\n  File \"/usr/local/lib/python3.9/site-packages/ansible_runner/runner.py\", line 106, in status_callback\n    ansible_runner.plugins[plugin].status_handler(self.config, status_data)\n  File \"/usr/local/lib/python3.9/site-packages/ansible_runner_http/events.py\", line 35, in status_handler\n    status = send_request(plugin_config['runner_url'],\n  File \"/usr/local/lib/python3.9/site-packages/ansible_runner_http/events.py\", line 18, in send_request\n    return session.post(url_actual, headers=headers, json=(data))\n  File \"/usr/local/lib/python3.9/site-packages/requests/sessions.py\", line 637, in post\n    return self.request(\"POST\", url, data=data, json=json, **kwargs)\n  File \"/usr/local/lib/python3.9/site-packages/requests/sessions.py\", line 589, in request\n    resp = self.send(prep, **send_kwargs)\n  File \"/usr/local/lib/python3.9/site-packages/requests/sessions.py\", line 703, in send\n    r = adapter.send(request, **kwargs)\n  File \"/usr/local/lib/python3.9/site-packages/requests/adapters.py\", line 637, in send\n    raise InvalidURL(e, request=request)\nrequests.exceptions.InvalidURL: Not supported URL scheme http+unix\n","job":"7062013831693068113","name":"bootstrap-token-abcdef","namespace":"kube-system","error":"exit status 1","stacktrace":"github.com/operator-framework/ansible-operator-plugins/internal/ansible/runner.(*runner).Run.func1\n\tansible-operator-plugins/internal/ansible/runner/runner.go:269"}

I'm not really sure how we work around this, since I'm not well versed in python.

I poised this question in k8s slack
https://kubernetes.slack.com/archives/C017UU45SHL/p1719326921876759

@Neo2308
Copy link
Author

Neo2308 commented Jun 25, 2024

@acornett21 Should we consider reverting the request package bump and ignore the security warning coming from it for now? By the way, really appreciate your help so far on this PR!

@acornett21
Copy link
Contributor

@Neo2308 After the discussion in slack, @everettraven agrees the path forward for now is to revert the ansible-core changes and ignore the CVE in both docker files. I believe the ID would be 71064.

@Neo2308 Neo2308 force-pushed the feature/main/bump-k8s-0.29.3 branch from 56d31fb to c5c8be1 Compare June 27, 2024 18:43
@Neo2308 Neo2308 force-pushed the feature/main/bump-k8s-0.29.3 branch from c5c8be1 to 2e0f3ec Compare June 27, 2024 18:44
@Neo2308
Copy link
Author

Neo2308 commented Jun 27, 2024

@acornett21 Made the changes. Could you review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants