-
Notifications
You must be signed in to change notification settings - Fork 67
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
Add changes to ensure azlinux and mcrypt images are working fine #2682
base: master
Are you sure you want to change the base?
Conversation
kube/services/jobs/usersync-job.yaml
Outdated
@@ -93,7 +93,7 @@ spec: | |||
name: "projects" | |||
containers: | |||
- name: usersync | |||
GEN3_FENCE_IMAGE | |||
GEN3_FENCE_MCRYPT_IMAGE |
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 think adding this, enforces all commons to have fence_mcrypt
in their manifest and doesn't seem like a great idea
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.
Yeah... It will force every job to use the new image for the usersync job regardless of if they're using the latest Fence, which is going to change what every Prod env runs without them explicitly updating a manifest or anything. We shouldn't do this. It doesn't allow someone to test the update in a staging/preprod before deploying in prod. We want to only use this new image if they're deploying the new Fence as configured in the manifest. Maybe we need a new manifest entry
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 suggested adding a new fence_mcrypt
image line in the manifest's versions, specifically for jobs where usersync requires this branch. The team would then need to manually update the usersync job in the AdminVM.
Need opinions.
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.
Approved assuming the pipeline issues are resolved. can you add a description to the PR so I or other reviewers have a better understanding of the purposes of these changes? The PR looks fine, but due to lack of context there may be things I am missing. Getting another review from someone else will also be beneficial.
Please find the ci env pod logs here |
Please find the ci env pod logs here |
Please find the ci env pod logs here |
Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
Add changes to fence related deploy yamls, to provide backwards compatibility with fence images using azlinux and old docker file with
python3.9-buster-2.0.0
image