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.

Sign up for GitHub

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

Jump to bottom

[openstackclient] Store hash to determine when pod spec changes #1211

Merged
merged 3 commits into from
Dec 7, 2024
Merged

[openstackclient] Store hash to determine when pod spec changes #1211

merged 3 commits into from
Dec 7, 2024

Conversation

olliewalsh
Copy link
Contributor

@olliewalsh olliewalsh commented Nov 28, 2024 •
edited
Loading

Comparing the desired pod spec to the current pod spec is not reliable as
the spec can be mutated by k8s e.g to add volume mount for a service account
or to add the default nodeselector from the namespace/cluster on OpenShift

Instead store a hash of the original pod spec and compare this to the hash
of the current desired pod spec to determine if the spec has changed.

Resolves: OSPRH-11701

@openshift-ci openshift-ci bot requested review from bshephar and jpodivin November 28, 2024 18:42
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 28, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: olliewalsh

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

@olliewalsh
Copy link
Contributor Author

/hold

@olliewalsh olliewalsh force-pushed the openstackclient-pod-template-hash branch from 65c93f8 to fba9c3a Compare November 28, 2024 21:14
@olliewalsh
Copy link
Contributor Author

/test openstack-operator-build-deploy-kuttl

1 similar comment
@olliewalsh
Copy link
Contributor Author

/test openstack-operator-build-deploy-kuttl

@olliewalsh olliewalsh force-pushed the openstackclient-pod-template-hash branch 2 times, most recently from 202d0f4 to 73dfdc9 Compare December 3, 2024 11:42
@olliewalsh olliewalsh changed the title DNM: test2 [openstackclient] Store hash to determine when pod spec changes Dec 3, 2024
@olliewalsh olliewalsh requested review from stuggi and abays December 3, 2024 11:43
@olliewalsh
Copy link
Contributor Author

/remove-hold

@olliewalsh
Copy link
Contributor Author

/cherry-pick 18.0-fr1

@openshift-cherrypick-robot
Copy link

@olliewalsh: once the present PR merges, I will cherry-pick it on top of 18.0-fr1 in a new PR and assign it to you.

In response to this:

/cherry-pick 18.0-fr1

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.

@olliewalsh olliewalsh requested a review from dprince December 5, 2024 14:11
@olliewalsh olliewalsh force-pushed the openstackclient-pod-template-hash branch from 73dfdc9 to 1f1eb2c Compare December 5, 2024 16:21
@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/openstack-operator for 1211,1f1eb2c6615c1ac69e9b90be28e0b54043749491

@olliewalsh
Set a namespace default nodeSelector for nodeSelector kuttl test
6b7917b
@olliewalsh
Store hash to determine when pod spec changes
5b6b54b 
Comparing the desired pod spec to the current pod spec is not reliable as
the spec can be mutated by k8s e.g to add volume mount for a service account
or to add the default nodeselector from the namespace/cluster on OpenShift

Instead store a hash of the original pod spec and compare this to the hash
of the current desired pod spec to determine if the spec has changed.
@olliewalsh
[crd-schema-checker] disable NoMaps validator
eaa330c 
Related: https://issues.redhat.com/browse/OSPRH-12254
@olliewalsh olliewalsh force-pushed the openstackclient-pod-template-hash branch from 1f1eb2c to eaa330c Compare December 5, 2024 16:25
@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/11995a064bad41f5af6c7be36a64e518

❌ openstack-k8s-operators-content-provider NODE_FAILURE Node request 100-0007693715 failed in 0s
⚠️ podified-multinode-edpm-deployment-crc SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ cifmw-crc-podified-edpm-baremetal SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ adoption-standalone-to-crc-ceph-provider SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ openstack-operator-tempest-multinode SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider

@olliewalsh
Copy link
Contributor Author

recheck

stuggi
stuggi reviewed Dec 6, 2024
View reviewed changes
Copy link
Contributor

@stuggi stuggi left a comment

Choose a reason for hiding this comment

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

lgtm

@stuggi stuggi added the lgtm label Dec 6, 2024
@olliewalsh
Copy link
Contributor Author

/test openstack-operator-build-deploy-kuttl

3 similar comments
@olliewalsh
Copy link
Contributor Author

/test openstack-operator-build-deploy-kuttl

@olliewalsh
Copy link
Contributor Author

/test openstack-operator-build-deploy-kuttl

@olliewalsh
Copy link
Contributor Author

/test openstack-operator-build-deploy-kuttl

@openshift-merge-bot openshift-merge-bot bot merged commit eba4d82 into openstack-k8s-operators:main Dec 7, 2024
8 checks passed
@openshift-cherrypick-robot
Copy link

@olliewalsh: #1211 failed to apply on top of branch "18.0-fr1":

Applying: Set a namespace default nodeSelector for nodeSelector kuttl test
Applying: Store hash to determine when pod spec changes
Applying: disable NoMaps validator
Using index info to reconstruct a base tree...
A	hack/crd-schema-checker.sh
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): hack/crd-schema-checker.sh deleted in HEAD and modified in disable NoMaps validator. Version disable NoMaps validator of hack/crd-schema-checker.sh left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0003 disable NoMaps validator

In response to this:

/cherry-pick 18.0-fr1

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.

@olliewalsh olliewalsh mentioned this pull request Dec 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuggi stuggi stuggi left review comments

@abays abays Awaiting requested review from abays

@dprince dprince Awaiting requested review from dprince

Assignees
No one assigned
Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@olliewalsh @openshift-cherrypick-robot @stuggi @openshift-merge-robot