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

Fixes issue with arm64 support. #404

Merged
merged 3 commits into from
Jan 16, 2023
Merged

Fixes issue with arm64 support. #404

merged 3 commits into from
Jan 16, 2023

Conversation

mprimeaux
Copy link
Contributor

@mprimeaux mprimeaux commented Dec 30, 2022 •
edited
Loading

Description

This PR adds support for multi-architecture container images. I am not sure of the CI flow but rather than reusing the existing make targets (e.g. docker-build), I could create new make targets for the docker builds commands but wanted to get the conversation started before making any assumptions.

Fixes #390

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • Testing has been performed
  • No functionality is broken
  • Documentation updated

@iamabhishek-dubey
Copy link
Member

@mprimeaux can we make sure CI is passing?

@mprimeaux
Copy link
Contributor Author

@iamabhishek-dubey Yes, of course. I will look into ensuring this passes.

@mprimeaux
Copy link
Contributor Author

mprimeaux commented Jan 5, 2023 •
edited
Loading

@iamabhishek-dubey Unless I am misreading the error, it appears this is a linting rule, which suggests not to use the --platform flag with FROM. However, this is needed when dealing with multi-architecture images.

Arguably, use of the --platform flag could be relaxed for on line 2 of the Dockerfile but not on line 47 since we need to specify $TARGETPLATFORM.

@mprimeaux
Copy link
Contributor Author

mprimeaux commented Jan 5, 2023 •
edited
Loading

@iamabhishek-dubey It appears the issue experiencing in the above CI pipelines has been updated to not be a linting error as per hadolint DL3029. Fixed in release 2.9.1.

Can the CI pipeline be updated to use the latest release? I can't find mention of hadolint in this repository. Your help is appreciated.

UPDATE: Perhaps this and this webhook are the references to hadolint that needs to be updated? Both are using 2.4.0.

This was referenced Jan 5, 2023
Closed
Closed
@mprimeaux
Copy link
Contributor Author

mprimeaux commented Jan 5, 2023 •
edited
Loading

@iamabhishek-dubey I've added two PRs in the k8s-vault-webhook and k8s-secret-injector repositories as per above. I'm unable to verify if this addresses the CI pipeline here but hope you can shed some insights on it.

However, the checks in those pipelines continually cancel. See here and here. Shouldn't the latter error use a more recent version of Ubuntu than 16?

Unfortunately, I don't have permissions to rerun any failed or cancelled jobs.

@iamabhishek-dubey
Copy link
Member

@mprimeaux I have updated the hadolint version for this CI pipeline as well but I can see it is still failing

@mprimeaux
Copy link
Contributor Author

@iamabhishek-dubey Thanks much. I am not sure about the pipeline failure, either. Here is the specific log from this pipeline.

You can see the failure *seems to be coming from hadolint. What is interesting is these are the same two warnings as before. Are these warnings being treated as errors?

@iamabhishek-dubey
Copy link
Member

I think it might be the case @mprimeaux

@iamabhishek-dubey
Copy link
Member

@mprimeaux any update on the PR?

@mprimeaux
Addresses issue
ad8f745 
[#390](#391) by
adding support for multi-architecture container images.

Signed-off-by: Michael Primeaux <[email protected]>
@mprimeaux
Removed --platform due to linting rule constraints.
5a40bad 
Signed-off-by: Michael Primeaux <[email protected]>
@mprimeaux
Copy link
Contributor Author

@iamabhishek-dubey Rather than relaxing the container image file lint rule for DL3029, I removed the --platform flag.

@mprimeaux
Copy link
Contributor Author

mprimeaux commented Jan 16, 2023 •
edited
Loading

Note that to build the multi-architecture container images, you'll want to first run the docker-create make target before running docker-build or bundle-build. I'm not sure of the release process.

@iamabhishek-dubey iamabhishek-dubey added the bug Something isn't working label Jan 16, 2023
@iamabhishek-dubey iamabhishek-dubey merged commit 8762e50 into OT-CONTAINER-KIT:master Jan 16, 2023
@milas milas mentioned this pull request Mar 1, 2023
Closed
devkmsg added a commit to devkmsg/redis-operator that referenced this pull request Mar 7, 2023
@devkmsg
Merge pull request OT-CONTAINER-KIT#11 in OSS/redis-operator from ~AT…
4a664bd 
…HOMPSON/redis-operator:sync-upstream-0.14 to master

Auto-Merge: Pull request OT-CONTAINER-KIT#11: [AUTO] Sync upstream @ v0.14

Merge in OSS/redis-operator from ~ATHOMPSON/redis-operatorsync-upstream-0.14 to master

* commit 'e86884ead1005484bdb10fb30caf8f8acac2f89b': (49 commits)
  [Feature] Add Redis Sentinel Support  (OT-CONTAINER-KIT#408)
  Fixed Redis Replicate Cache bug (OT-CONTAINER-KIT#424)
  [Feature] : Add Replication Mode to the Redis Operator (OT-CONTAINER-KIT#417)
  [Development][Add] Added recreation logic for statefulset (OT-CONTAINER-KIT#411)
  Fixes issue with arm64 support. (OT-CONTAINER-KIT#404)
  [Development][Add] Added nodeSelector and tolerations for cluster (OT-CONTAINER-KIT#410)
  Add Label Selector to pod anti affinity  (OT-CONTAINER-KIT#407)
  When cr annotation update,sts annotations will not updated! (OT-CONTAINER-KIT#398)
  fix: invalid memory address or nil pointer dereference (OT-CONTAINER-KIT#395)
  export redis exporter as a container port (OT-CONTAINER-KIT#393)
  [Development][Add] Added feature for additional volume mounts (OT-CONTAINER-KIT#389)
  fix crash with go panic (OT-CONTAINER-KIT#385)
  Add check PersistenceEnabled not nil (OT-CONTAINER-KIT#380)
  [feature]add serviceType functionality for standalone and cluster with annotations (OT-CONTAINER-KIT#376)
  [Development][Update]Updated information for v0.13.0 (OT-CONTAINER-KIT#374)
  Create CODE_OF_CONDUCT.md
  [feature]add tls for redis-standlone (OT-CONTAINER-KIT#372)
  Update README.md
  Create package.json
  Revamped documentation for better knowledge base (OT-CONTAINER-KIT#370)
  ...
devkmsg added a commit to devkmsg/redis-operator that referenced this pull request Jan 30, 2024
@devkmsg
Pull request OT-CONTAINER-KIT#12: Sync our internal branch to v0.14 w…
8b233c9 
…/internal patches

Merge in OSS/redis-operator from ~ATHOMPSON/redis-operator:sync-internal-cs-main-to-0.14 to cs-main

* commit '2ea8fcaf61b322186f8a0a2c4e7bcb310f55ea2d':
  Revert "Handle nil probe"
  Handle nil probe
  Bumps prometheus/client_golang to address vuln
  Adds CODEOWNERS for our internal branch
  [Feature] Add Redis Sentinel Support  (OT-CONTAINER-KIT#408)
  Fixed Redis Replicate Cache bug (OT-CONTAINER-KIT#424)
  [Feature] : Add Replication Mode to the Redis Operator (OT-CONTAINER-KIT#417)
  [Development][Add] Added recreation logic for statefulset (OT-CONTAINER-KIT#411)
  Fixes issue with arm64 support. (OT-CONTAINER-KIT#404)
  [Development][Add] Added nodeSelector and tolerations for cluster (OT-CONTAINER-KIT#410)
  Add Label Selector to pod anti affinity  (OT-CONTAINER-KIT#407)
  When cr annotation update,sts annotations will not updated! (OT-CONTAINER-KIT#398)
  fix: invalid memory address or nil pointer dereference (OT-CONTAINER-KIT#395)
  export redis exporter as a container port (OT-CONTAINER-KIT#393)
  [Development][Add] Added feature for additional volume mounts (OT-CONTAINER-KIT#389)
  fix crash with go panic (OT-CONTAINER-KIT#385)
  Add check PersistenceEnabled not nil (OT-CONTAINER-KIT#380)
  [feature]add serviceType functionality for standalone and cluster with annotations (OT-CONTAINER-KIT#376)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iamabhishek-dubey iamabhishek-dubey iamabhishek-dubey approved these changes

Assignees

@mprimeaux mprimeaux

Labels
bug Something isn't working
Projects
Redis Operator Project Board
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

arm64 support not working
2 participants
@mprimeaux @iamabhishek-dubey