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

Fix ImageChange triggers not being set in DeploymentConfig when resource fragments are used. #1794

Merged

Conversation

rohanKanojia
Copy link
Member

Related StackOverlow post: https://stackoverflow.com/questions/60375063

This seems to be a regression since we introduced sidecar containers support. For applying
ImageChange triggers we have to check which containers are main application containers and
which are sidecars. Well, we do that but when we don't set any defaultApplicationContainerName
when the sidecar option is disabled. It seemed like a bug introduced me during implementation.

Sample project for testing: https://github.com/r0haaaan/fmp-openshift-sample-with-fragments

If you test on master, you can see in generated resources that ImageChange triggers are not being applied. But on this patch, they are being applied correctly.

…rce fragments are used.

Related StackOverlow post: https://stackoverflow.com/questions/60375063

This seems to be a regression since we introduced sidecar containers support. For applying
ImageChange triggers we have to check which containers are main application containers and
which are sidecars. Well, we do that but when we don't set any `defaultApplicationContainerName`
when sidecar option is disabled. It seemed like a bug introduced my me during implementation.
@rohanKanojia rohanKanojia merged commit 6525f61 into fabric8io:master Mar 2, 2020
rohanKanojia added a commit to rohanKanojia/jkube that referenced this pull request Mar 6, 2020
rohanKanojia added a commit to rohanKanojia/jkube that referenced this pull request Mar 9, 2020
manusa pushed a commit to eclipse-jkube/jkube that referenced this pull request Mar 9, 2020
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