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

peer-pods: validate CM and Secret are set #353

Merged
merged 3 commits into from Nov 9, 2023

Conversation

snir911
Copy link
Contributor

@snir911 snir911 commented Nov 7, 2023

No description provided.

@snir911 snir911 self-assigned this Nov 7, 2023
Copy link
Member

@gkurz gkurz left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @snir911 !

@snir911 snir911 added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 8, 2023
@snir911 snir911 force-pushed the 2557-devel-fix branch 2 times, most recently from 4b751a4 to b209fd4 Compare November 8, 2023 12:37
@snir911 snir911 requested a review from gkurz November 8, 2023 15:34
@snir911 snir911 removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 8, 2023
@cpmeadors
Copy link
Contributor

Premerge testing successful.

@gkurz
Copy link
Member

gkurz commented Nov 9, 2023

Hi @snir911 !

I had approved your original submission as the change was simple to understand, despite the lack of change log. I must admit that now things are a lot harder... Not sure if I'll be able to approve this PR today if I need to guess everything by myself.

controllers/image_generator.go Show resolved Hide resolved
controllers/image_generator.go Show resolved Hide resolved
controllers/image_generator.go Show resolved Hide resolved
We usually fetching cloud-provider name from either the peer-pods CM CLOUD_PROVIDER value
or from the infra object, as cloud provider is not expected to be changed at runtime
this commit moves its fetching into the image-generator initialization.
Additionally fetching the cloud-provider solely from the infra Object to avoid relying on
user input (from peer-pods CM)

Signed-off-by: Snir Sheriber <ssheribe@redhat.com>
in peer-pods ConfigMap or Secret
it's assumed essential values will not be modified once set and
kataConfig was created

Signed-off-by: Snir Sheriber <ssheribe@redhat.com>
Signed-off-by: Snir Sheriber <ssheribe@redhat.com>
Copy link
Member

@gkurz gkurz left a comment

Choose a reason for hiding this comment

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

Thanks for the work and patience @snir911 !

@gkurz gkurz merged commit 7c3e212 into openshift:devel Nov 9, 2023
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

3 participants