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

adds support in scripts for deploying nephio components on real K8S(non-kind) #269

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

vjayaramrh
Copy link
Collaborator

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test

kind feature

/kind flake

What this PR does / why we need it:
This PR allows for deploying nephio management components on pre-existing K8S cluster using the existing scripts. On a machine with K8S installed, one would be able to execute the below command with correct values to install Nephio management components.

wget -O - https://raw.githubusercontent.com/nephio-project/test-infra/v2.0.0/e2e/provision/init.sh | sudo NEPHIO_DEBUG=false NEPHIO_BRANCH=v2.0.0 NEPHIO_USER=ubuntu K8S_CONTEXT=kubernetes-admin@kubernetes bash

The value for K8S_CONTEXT can be obtained by executing the below command
kubectl config get-contexts

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


@vjayaramrh
Copy link
Collaborator Author

/retest

Copy link
Member

@liamfallon liamfallon left a comment

Choose a reason for hiding this comment

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

/approve

@vjayaramrh vjayaramrh force-pushed the non_kind_k8s branch 2 times, most recently from 0f55cab to 1a1136a Compare June 19, 2024 12:42
@vjayaramrh
Copy link
Collaborator Author

/retest bootstrap-integration

Copy link
Contributor

nephio-prow bot commented Jun 20, 2024

@vjayaramrh: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test bootstrap-integration
  • /test e2e-free5gc-ubuntu-focal
  • /test e2e-oai-ubuntu-jammy
  • /test images-hadolint
  • /test install-integration
  • /test kpt-integration
  • /test pre-submit-test-infra-validate-local
  • /test presubmit-test-infra-inrepoconfig-validation
  • /test provision-linter

The following commands are available to trigger optional jobs:

  • /test e2e-free5gc-fedora-34
  • /test e2e-oai-fedora-34

Use /test all to run the following jobs that were automatically triggered:

  • bootstrap-integration
  • e2e-free5gc-fedora-34
  • e2e-free5gc-ubuntu-focal
  • e2e-oai-fedora-34
  • e2e-oai-ubuntu-jammy
  • provision-linter

In response to this:

/retest bootstrap-integration

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@vjayaramrh
Copy link
Collaborator Author

/retest provision-linter

Copy link
Contributor

nephio-prow bot commented Jun 20, 2024

@vjayaramrh: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test bootstrap-integration
  • /test e2e-free5gc-ubuntu-focal
  • /test e2e-oai-ubuntu-jammy
  • /test images-hadolint
  • /test install-integration
  • /test kpt-integration
  • /test pre-submit-test-infra-validate-local
  • /test presubmit-test-infra-inrepoconfig-validation
  • /test provision-linter

The following commands are available to trigger optional jobs:

  • /test e2e-free5gc-fedora-34
  • /test e2e-oai-fedora-34

Use /test all to run the following jobs that were automatically triggered:

  • bootstrap-integration
  • e2e-free5gc-fedora-34
  • e2e-free5gc-ubuntu-focal
  • e2e-oai-fedora-34
  • e2e-oai-ubuntu-jammy
  • provision-linter

In response to this:

/retest provision-linter

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@vjayaramrh
Copy link
Collaborator Author

/test provision-linter

@vjayaramrh
Copy link
Collaborator Author

/test bootstrap-integration

@vjayaramrh
Copy link
Collaborator Author

/test e2e-oai-fedora-34

@vjayaramrh
Copy link
Collaborator Author

@electrocucaracha I see the below in the bootstrap-integration, how do I fix this? THanks

TASK [Include bootstrap] ******************************************************* ERROR! 'REAL_K8S_TAG' is undefined. 'REAL_K8S_TAG' is undefined

export ANSIBLE_CMD_EXTRA_VAR_LIST="nephio_catalog_repo_uri='$NEPHIO_CATALOG_REPO_URI'"
K8S_CONTEXT=${K8S_CONTEXT:-"kind-kind"}
K8S_VERSION=${K8S_VERSION:-"v1.29.2"}
export ANSIBLE_CMD_EXTRA_VAR_LIST='{ "nephio_catalog_repo_uri": "'${NEPHIO_CATALOG_REPO_URI}'", "k8s": { "context" : "'${K8S_CONTEXT}'", "version" : "'$K8S_VERSION'" } }'
Copy link
Member

Choose a reason for hiding this comment

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

@vjayaramrh ANSIBLE_CMD_EXTRA_VAR_LIST variable can be provided by the user, so this overrides its value

Suggested change
export ANSIBLE_CMD_EXTRA_VAR_LIST='{ "nephio_catalog_repo_uri": "'${NEPHIO_CATALOG_REPO_URI}'", "k8s": { "context" : "'${K8S_CONTEXT}'", "version" : "'$K8S_VERSION'" } }'
export ANSIBLE_CMD_EXTRA_VAR_LIST="${ANSIBLE_CMD_EXTRA_VAR_LIST:-'{ "nephio_catalog_repo_uri": "'${NEPHIO_CATALOG_REPO_URI}'", "k8s": { "context" : "'${K8S_CONTEXT}'", "version" : "'$K8S_VERSION'" } }'}"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, thanks

Copy link
Collaborator Author

@vjayaramrh vjayaramrh Jun 21, 2024

Choose a reason for hiding this comment

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

The original code also did not account for user provided value for ANSIBLE_CMD_EXTRA_VAR_LIST. We should treat user provided input for this separately as we need to test out and validate other cases.
The good thing is I have tested out case where multiple --extra-vars can be supplied to a playbook and this can be used for future implementation.

Copy link
Member

Choose a reason for hiding this comment

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

This env variable is used by Vagrant, so overriding its value will result in a misfunctioning. Could you find a way to avoid that misfunctioning?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@electrocucaracha Thanks for the comment, I plan to open an issue to be fixed as part of a later PR. Would that be OK?. I noticed that you have 'https://github.com/nephio-project/nephio-catalog-packages.git in the link above that is not valid. Is that by design or should it have been https://github.com/nephio-project/catalog.git?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FYI I have opened an issue to address this post R3 at nephio-project/nephio#777

Comment on lines 82 to 88
if [ ${K8S_CONTEXT} == "kind-kind" ]; then
export ANSIBLE_TAG=all
else
export ANSIBLE_TAG=real_k8s
fi
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if [ ${K8S_CONTEXT} == "kind-kind" ]; then
export ANSIBLE_TAG=all
else
export ANSIBLE_TAG=real_k8s
fi
ANSIBLE_TAG=all
if [ ${K8S_CONTEXT:-"kind-kind"} != "kind-kind" ]; then
ANSIBLE_TAG=production
fi

Copy link
Collaborator Author

@vjayaramrh vjayaramrh Jun 20, 2024

Choose a reason for hiding this comment

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

Good suggestion.
The value for ANSIBLE_TAG value will be kept as real_k8s for readability

@@ -12,6 +12,7 @@
hosts: all
vars:
container_engine: docker
tag_real_k8s: real_k8s
Copy link
Member

Choose a reason for hiding this comment

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

What about to using another name?

Suggested change
tag_real_k8s: real_k8s
k8s_type: production

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good suggestion, I like tag_k8s_type. I prefer value to be real_k8s as against production for the time being.

Copy link
Collaborator Author

@vjayaramrh vjayaramrh Jun 21, 2024

Choose a reason for hiding this comment

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

After thinking about it some more, decided to keep the original name as this is more like a constant and will always have the same value.

Copy link
Member

Choose a reason for hiding this comment

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

@liamfallon @radoslawc @arora-sagar any suggestion on this? My preference is to avoid the usage of real on these tags, but I'd like to hear other point of views.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@electrocucaracha what do you think about tag_nonkind_k8s: nonkind_k8s ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@electrocucaracha I agree with you, even I don't like the term real and neither kind or non-kind because the same kind cluster we can make with minikube.

so how about k8s_distribution ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am thinking better name would be pre_deployed_k8s or similar instead of nonkind_k8s. The tag identifies the code sections that need to be executed on an existing cluster and skip the other parts(installing docker, kind and such). Will push a PR later with some of the feedback once this merges as we are close to code freeze and regularly failing e2e tests are not helping the cause.

@electrocucaracha
Copy link
Member

@electrocucaracha I see the below in the bootstrap-integration, how do I fix this? THanks

TASK [Include bootstrap] ******************************************************* ERROR! 'REAL_K8S_TAG' is undefined. 'REAL_K8S_TAG' is undefined

That's because it is not defined on the molecule tests, btw could you use another lower case name, I was thinking in type, kind, environment, non-kind, non-existing

@liamfallon
Copy link
Member

/retest

@electrocucaracha
Copy link
Member

@liamfallon the bootstrap-integration failure is genuine, it has to be fixed prior to its merge

@liamfallon
Copy link
Member

/retest

@liamfallon the bootstrap-integration failure is genuine, it has to be fixed prior to its merge

OK, thanks @electrocucaracha

@vjayaramrh vjayaramrh force-pushed the non_kind_k8s branch 2 times, most recently from c0f7efb to ea891f9 Compare June 26, 2024 00:33
@vjayaramrh
Copy link
Collaborator Author

/test e2e-free5gc-ubuntu-focal

@vjayaramrh
Copy link
Collaborator Author

/test e2e-oai-ubuntu-jammy

@vjayaramrh
Copy link
Collaborator Author

/test e2e-free5gc-fedora-34

@vjayaramrh
Copy link
Collaborator Author

/test e2e-oai-ubuntu-jammy

1 similar comment
@liamfallon
Copy link
Member

/test e2e-oai-ubuntu-jammy

@liamfallon
Copy link
Member

liamfallon commented Jun 26, 2024

"Deploy OAI Core Network Functions" on oai-amf failed in the e2e-oai-ubuntu-jimmy test, will retry:
/test e2e-oai-ubuntu-jammy

@liamfallon
Copy link
Member

"Deploy OAI Core Network Functions" failed in the e2e-oai-ubuntu-jimmy test on "upf-edge" NF

@liamfallon
Copy link
Member

/test e2e-oai-ubuntu-jammy

1 similar comment
@liamfallon
Copy link
Member

/test e2e-oai-ubuntu-jammy

@vjayaramrh
Copy link
Collaborator Author

/test e2e-oai-ubuntu-jammy

THis time it seems to have failed deploying oai-smf, I guess

@vjayaramrh
Copy link
Collaborator Author

/test e2e-oai-ubuntu-jammy

@arora-sagar
Copy link
Contributor

@vjayaramrh Is it working locally for you?

@vjayaramrh
Copy link
Collaborator Author

@vjayaramrh Is it working locally for you?

I have not tried running the e2e locally, the same test passed CI earlier.

@liamfallon
Copy link
Member

/test e2e-oai-ubuntu-jammy

@vjayaramrh
Copy link
Collaborator Author

@vjayaramrh Is it working locally for you?

@arora-sagar its now working locally

@vjayaramrh
Copy link
Collaborator Author

/test e2e-free5gc-fedora-34

Copy link
Contributor

nephio-prow bot commented Jun 26, 2024

@vjayaramrh: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
e2e-free5gc-fedora-34 8157350 link false /test e2e-free5gc-fedora-34

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@liamfallon
Copy link
Member

/approve
/lgtm

Copy link
Contributor

nephio-prow bot commented Jun 26, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liamfallon

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:

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

@nephio-prow nephio-prow bot merged commit 2ccf753 into nephio-project:main Jun 26, 2024
6 of 8 checks passed
nephio-prow bot pushed a commit to nephio-project/docs that referenced this pull request Jun 27, 2024
This is contingent on, and related to
nephio-project/test-infra#269 so will be
submitted as a draft for the time being. Documenting the additional
variable of K8S_CONTEXT introduced in the linked PR for deploying Nephio
components on non-kind clusters.

Note also that in the "Real K8s Cluster" installation instructions the
branch should be updated once
nephio-project/test-infra#269 is merged and a
new tag is available.

Closes nephio-project/nephio#577

---------

Co-authored-by: Liam Fallon <[email protected]>
@vjayaramrh vjayaramrh deleted the non_kind_k8s branch July 18, 2024 20:33
dkosteck added a commit to vjayaramrh/test-infra that referenced this pull request Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants