-
Notifications
You must be signed in to change notification settings - Fork 36
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
Adding ARM testing in E2E test suite #46
Conversation
Dockerfile
Outdated
# We need the full version of GnuPG | ||
RUN dnf install -y --allowerasing wget gnupg2 | ||
|
||
RUN MP_ARCH=`uname -p | sed s/aarch64/arm64/` && \ | ||
RUN echo ${BUILDPLATFORM} | ||
RUN echo ${TARGETARCH} |
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.
These two echos can be removed
Dockerfile
Outdated
ARG TARGETARCH | ||
ARG TARGETPLATFORM | ||
RUN uname -a | ||
RUN echo $TARGETPLATFORM |
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.
Lines 48-50 can be removed (TARGETPLATFORM isn't used in this stage)
Dockerfile
Outdated
ARG MOUNTPOINT_VERSION | ||
ARG TARGETARCH | ||
ARG TARGETPLATFORM |
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.
Also not needed (both of these ARG
s)
Dockerfile
Outdated
ENV MOUNTPOINT_VERSION=${MOUNTPOINT_VERSION} | ||
|
||
RUN yum install util-linux -y | ||
# RUN yum install util-linux -y |
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.
Delete
hack/provenance.sh
Outdated
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
# There is no reliable way to check if a buildx installation supports |
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.
How do you feel about changing this directory from hack
to something like scripts
? And we can throw the mp installer script in there too. Minor thing but I don't love the connotation of hack
(I know some of the other drivers do this).
CLUSTER_FILE=${TEST_DIR}/${CLUSTER_NAME}.${CLUSTER_TYPE}.yaml | ||
KOPS_PATCH_FILE=${KOPS_PATCH_FILE:-${BASE_DIR}/kops-patch.yaml} | ||
KOPS_PATCH_NODE_FILE=${KOPS_PATCH_NODE_FILE:-${BASE_DIR}/kops-patch-node.yaml} | ||
KOPS_STATE_FILE=${KOPS_STATE_FILE:-s3://mountpoint-s3-csi-driver-kops-state-store} | ||
|
||
KOPS_STATE_FILE_ARM=${KOPS_STATE_FILE_ARM:-s3://mountpoint-s3-csi-driver-kops-arm-state-store} |
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.
It might be cleaner to conditionally set a couple of these vars (e.g. just set KOPS_STATE_FILE to something that includes $ARCH in the string). I think you could cut down on the duplication later in the files.
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.
thought about doing this earlier as well, since the S3 bucket is manually created, I wanted to leave the names for it hard coded (similar to how we're specifying the cluster name hard coded if it's eksctl since that was manually created)
Description of changes:
Note
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.