Skip to content

Conversation

@acornett21
Copy link
Contributor

Issue #, if available:

Description of changes:
This pull request is needed to get tests to pass for

It also removes sha information from k8's versions, since this sha version is not static. Meaning that k8s can and does update a sha for a given version to fix security updates and some of these shas can no longer be pulled locally.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Signed-off-by: Adam D. Cornett adc@redhat.com

…ng `kubectl apply` on the RBAC folder, since a service account is now being created in that folder

Signed-off-by: Adam D. Cornett <adc@redhat.com>
done
echo "ok."

echo -n "creating ack-system namespace"
Copy link
Contributor

Choose a reason for hiding this comment

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

One questions:

  1. deployment.yaml file in kustomize tries to kubectl apply ack-system namespace again? Did you any error/warning for that when running test locally?

Copy link
Contributor

Choose a reason for hiding this comment

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

Kustomize doesn't care - it just overwrites it with the same value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vijtrip2 What @RedbackThomson said is correct, since this is creating the namespace the same way as in the deployment.yaml there aren't any issues/warnings/errors. Below are the logs for clarity, meant to add them to the PR but forgot.

creating ack-system namespacenamespace/ack-system created
loading RBAC manifests for s3 into the cluster ... ok.
loading service controller Deployment for s3 into the cluster ...ok.

Copy link
Contributor

@vijtrip2 vijtrip2 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

apiVersion: kind.x-k8s.io/v1alpha4
nodes:
- role: control-plane
kubeadmConfigPatches:
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason for removing this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Adam and I were trying to debug an issue with his KIND cluster not being created properly. I noticed this unnecessary config patch for the cluster and we deleted it just to make sure it wasn't affecting him. It's not necessary for the e2e tests, so we figured we'd just leave it out.

K8_1_16="kindest/node:v1.16.9@sha256:7175872357bc85847ec4b1aba46ed1d12fa054c83ac7a8a11f5c268957fd5765"
K8_1_15="kindest/node:v1.15.11@sha256:6cc31f3533deb138792db2c7d1ffc36f7456a06f1db5556ad3b6927641016f50"
K8_1_14="kindest/node:v1.14.10@sha256:6cd43ff41ae9f02bb46c8f455d5323819aec858b99534a290517ebc181b443c6"
K8_1_21="kindest/node:v1.21.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@RedbackThomson RedbackThomson changed the title updating kind-build-test.sh file to create a namespace before executing kubectl apply on the RBAC folder Create ack-system namespace before applying RBAC test resources Jan 25, 2022
@RedbackThomson
Copy link
Contributor

/lgtm

@ack-bot ack-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 25, 2022
@ack-bot
Copy link
Collaborator

ack-bot commented Jan 25, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: acornett21, RedbackThomson, vijtrip2

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [RedbackThomson,vijtrip2]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-bot ack-bot merged commit 8d624d5 into aws-controllers-k8s:main Jan 25, 2022
@acornett21 acornett21 deleted the create_namespace branch January 25, 2022 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants