-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
mgr/k8sevents: update V1Events to CoreV1Events #57253
mgr/k8sevents: update V1Events to CoreV1Events #57253
Conversation
46ed021
to
69a8c46
Compare
src/pybind/mgr/k8sevents/module.py
Outdated
|
||
# client.V1Event is not available on kubernetes>19.x. Use CoreV1Event instead | ||
# since centos8 only provides kubernetes==11.x, we'll need to support both | ||
# implementations | ||
try: | ||
return client.CoreV1Event( | ||
involved_object=obj_ref, | ||
metadata=obj_meta, | ||
message=self.message, | ||
count=self.count, | ||
type=self.event_type, | ||
reason=self.event_reason, | ||
source=event_source, | ||
first_timestamp=self.first_timestamp, | ||
last_timestamp=self.last_timestamp | ||
) | ||
except AttributeError: | ||
return client.V1Event( | ||
involved_object=obj_ref, | ||
metadata=obj_meta, | ||
message=self.message, | ||
count=self.count, | ||
type=self.event_type, | ||
reason=self.event_reason, | ||
source=event_source, | ||
first_timestamp=self.first_timestamp, | ||
last_timestamp=self.last_timestamp | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, but here, you're still referring to CoreV1Event
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems as though this block isn't needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated, thanks
69a8c46
to
9dbd9d7
Compare
centos9 only provides kubernetes 26.1.0 as base dep and hence the k8sevents code needs to be updated accordingly. the api changes happened in kuberenetes while 19.0.0 was released Fixes: https://tracker.ceph.com/issues/65627 Fixes: https://tracker.ceph.com/issues/64981 Signed-off-by: Nizamudeen A <nia@redhat.com>
9dbd9d7
to
6af9647
Compare
@rkachach: I have created two images for testing this on rook. centos9: docker.io/nizamial09/ceph:k8s_events_fix-c9 You can use this to start a rook cluster and then enable the k8sevents module and check the You can trigger an event like setting some osd flags; More info are in the README file of that module. |
cc: @jmolmo ^ |
What's our status here? This is blocking ceph on centos9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have the big picture to understand what's the scope of the impact of this change but as of Nizam's testing the change seems to be working properly with a newer version of kubernetes (python lib). Overall, LGTM.
I have opened this tracker to see if it can be deprecated. https://tracker.ceph.com/issues/66091, meanwhile I am going to merge this. Maybe we will need to send a mail to users list to see if someone is using this module. |
@dmick encountered this issue on centos 9 container build: https://tracker.ceph.com/issues/65627
centos9 only provides kubernetes 26.1.0 as base dep and hence the k8sevents code needs to be updated accordingly. the api changes happened in kuberenetes while 19.0.0 was released
Contribution Guidelines
To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an
x
between the brackets:[x]
. Spaces and capitalization matter when checking off items this way.Checklist
Show available Jenkins commands
jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e