Skip to content
This repository has been archived by the owner on Dec 21, 2021. It is now read-only.

Crash fixed when no tag is provided for the container image #40

Merged
merged 3 commits into from
Jan 22, 2021
Merged

Crash fixed when no tag is provided for the container image #40

merged 3 commits into from
Jan 22, 2021

Conversation

siegfriedweber
Copy link
Member

Fixes #39

When no tag is provided for the container image then the agent crashes:

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', src/provider/repository/package.rs:42:35

With this change the pod is rejected with an according log message:

2021-01-20T14:18:58Z WARN  kubelet::kubelet] Error resyncing pods: Invalid content in pod object: Unable to get package reference from pod "kafka": Tag is required.

@CLAassistant
Copy link

CLAassistant commented Jan 20, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

soenkeliebau
soenkeliebau previously approved these changes Jan 20, 2021
View reviewed changes
Copy link
Member

@soenkeliebau soenkeliebau left a comment

Choose a reason for hiding this comment

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

Looks very good to me!

@soenkeliebau
Copy link
Member

I spoke with Lars this morning and he will try to review this PR this afternoon.

lfrancke
lfrancke suggested changes Jan 21, 2021
View reviewed changes
Copy link
Member

@lfrancke lfrancke left a comment

Choose a reason for hiding this comment

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

Only minor stuff.
This looks fabulous.
If this is your first Rust PR I'm depressed :) Good job!

src/provider/repository/package.rs Outdated Show resolved Hide resolved
src/provider/mod.rs Outdated Show resolved Hide resolved
src/provider/mod.rs Outdated Show resolved Hide resolved
src/provider/mod.rs Show resolved Hide resolved
Cargo.toml Show resolved Hide resolved
@lfrancke lfrancke added this to In progress in Stackable Jan 21, 2021
Stackable automation moved this from In progress to Review in progress Jan 22, 2021
lfrancke
lfrancke approved these changes Jan 22, 2021
View reviewed changes
Copy link
Member

@lfrancke lfrancke left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you!

Stackable automation moved this from Review in progress to To be merged Jan 22, 2021
@lfrancke lfrancke merged commit 1c3c34f into stackabletech:main Jan 22, 2021
Stackable automation moved this from To be merged to Done Jan 22, 2021
@siegfriedweber siegfriedweber deleted the fix_crash_on_missing_reference_tag branch January 22, 2021 13:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lfrancke lfrancke lfrancke approved these changes

@soenkeliebau soenkeliebau soenkeliebau left review comments

Assignees

@siegfriedweber siegfriedweber

Labels
None yet
Projects
No open projects
Stackable
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Crash if no tag is provided for the container image
4 participants
@siegfriedweber @CLAassistant @soenkeliebau @lfrancke