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

Feat/ipv6only #212

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

mdrahman-suse
Copy link
Contributor

Proposed Changes

Types of Changes

Testing

Checklist:

  1. If your PR changes anything on or related to Jenkins, run it pointing to your branch to make sure it's okay.

  2. Verify code lint; we should not have errors.

  3. Update the documentation if needed.

  4. Update makefile and docker run if adding new tests.

  5. Run your tests at least 4 times with all configurations needed and possible.

  6. If needed test with different os types.

Linked Issues

Further Comments

@mdrahman-suse mdrahman-suse marked this pull request as ready for review February 7, 2025 19:29
@mdrahman-suse mdrahman-suse force-pushed the feat/ipv6only branch 5 times, most recently from 008f906 to 9c71ec7 Compare February 10, 2025 13:46
@@ -120,3 +120,4 @@ pre-commit:
@go vet ./...
@golangci-lint run --tests ./...
@shellcheck modules/airgap/setup/*.sh
@shellcheck modules/ipv6only/scripts/*.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

can we create a make shell check ? just to not start getting bigger and bigger this one

shared/airgap.go Outdated
// DisplayAirgapClusterDetails executes and prints kubectl get nodes,pods on bastion.
func DisplayAirgapClusterDetails(cluster *Cluster) {
// LogClusterDetailsViaProxy executes and prints kubectl get nodes,pods on bastion.
func LogClusterDetailsViaProxy(cluster *Cluster) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we name more accurately , idk something like LogClusterDetailsFromBastion ?
although it behaves like a proxy , but i think can bring confusion

func TestIPv6Only(cluster *shared.Cluster, awsClient *aws.Client) {
shared.LogLevel("info", "Setting up %s cluster on ipv6 only nodes...", cluster.Config.Product)
err := support.ConfigureIPv6OnlyNodes(cluster, awsClient)
Expect(err).NotTo(HaveOccurred(), err)
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add a break line here? after your expect >?

// TestAirgapClusterPodStatus test the status of the pods in the private cluster using custom assert functions.
func TestAirgapClusterPodStatus(
// TestPodStatusViaProxy test the status of the pods in the private cluster using custom assert functions.
func TestPodStatusViaProxy(
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment from Log via proxy

// TestAirgapClusterNodeStatus test the status of the nodes in the private cluster using 2 custom assert functions.
func TestAirgapClusterNodeStatus(
// TestNodeStatusViaProxy test the status of the nodes in the private cluster using 2 custom assert functions.
func TestNodeStatusViaProxy(
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment from Log via proxy

@mdrahman-suse mdrahman-suse force-pushed the feat/ipv6only branch 3 times, most recently from 28c9ee1 to 15b4391 Compare February 10, 2025 21:55
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.

3 participants