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

Get Cluster Information for Running FA on AWS and Test Nodes Status #32

Merged
merged 10 commits into from
May 7, 2023
Merged

Get Cluster Information for Running FA on AWS and Test Nodes Status #32

merged 10 commits into from
May 7, 2023

Conversation

razo7
Copy link
Member

@razo7 razo7 commented Apr 2, 2023 •
edited
Loading

Get cluster information for running Fence Agent (FA) on AWS (and partially for BMH cluster platform) and test node's status.

We now add a non-dummy FA, fence_aws for AWS clusters or fence_ipmilan for Bare Metal cluster, to test it in CI. ATM we support AWS cluster, thus only fence_aws has been tested.

This PR uses Kubernetes resources to get cluster information for running the fence agent:

  • Cluster Type
  • For AWS clusterss - AWS Region
  • Credentials : access key and access id

In this PR we only test fence_aws on CI with status action. and #20 will address the reboot/remediation action.

@openshift-ci openshift-ci bot requested review from beekhof and mshitrit April 2, 2023 11:26
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 2, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: razo7

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

@openshift-ci openshift-ci bot added the approved label Apr 2, 2023
@razo7
Copy link
Member Author

razo7 commented Apr 3, 2023

/retest

@razo7 razo7 force-pushed the get-cluster-info-e2e branch 3 times, most recently from aeedc7a to 33f2707 Compare April 4, 2023 10:43
@razo7
Copy link
Member Author

razo7 commented Apr 4, 2023

It seems like the PR helped to fetch the AWS cluster information, since it didn't fail in the process, but running the CLI command was denied (1.680609830558035e+09 ERROR executer Failed to run exec command {"com....

I need to follow up after Passover to better understand from FAR's logs if it has failed due to wrong call of fence_aws with it's parameters or a permission issue...

@razo7 razo7 force-pushed the get-cluster-info-e2e branch 3 times, most recently from 1a11b1e to 097ae00 Compare May 1, 2023 09:33
@razo7 razo7 changed the title [WIP] Get cluster info E2E [WIP] Get Cluster Information for Running FA on AWS and Test Nodes Status May 1, 2023
@razo7 razo7 changed the title [WIP] Get Cluster Information for Running FA on AWS and Test Nodes Status Get Cluster Information for Running FA on AWS and Test Nodes Status May 1, 2023
@razo7
Copy link
Member Author

razo7 commented May 2, 2023

/retest

clobrano
clobrano reviewed May 2, 2023
View reviewed changes
.vscode/launch.json Outdated Show resolved Hide resolved
controllers/fenceagentsremediation_controller_test.go Outdated Show resolved Hide resolved
test/e2e/far_e2e_test.go Show resolved Hide resolved
test/e2e/far_e2e_test.go Show resolved Hide resolved
test/e2e/utils/cluster.go Outdated Show resolved Hide resolved
test/e2e/utils/cluster.go Outdated Show resolved Hide resolved
@razo7 razo7 force-pushed the get-cluster-info-e2e branch 2 times, most recently from ee98e22 to 93772c3 Compare May 2, 2023 11:06
@razo7
Copy link
Member Author

razo7 commented May 2, 2023

/retest

@razo7
Get cluster information for AWS E2E
c7314ae 
Based on the fetched infromation from OpenShift CI we run the fence_aws FA
razo7 added 2 commits May 2, 2023 17:37
@razo7
Add e2e test - FAR logs
1d3aed4 
Check the FAR logs that indicate a successful CLI executation to verify that the fence agent has been running successfully
@razo7
Build shared and node parameters for FAR CR based on cluster type
2c6e157 
Automate FA selection and creation based on the platform it use - AWS or BMH
razo7 added 3 commits May 2, 2023 17:38
@razo7
Run fence_aws with status on far-install namespace
e7caa97 
CRs are namepsaced, thus they should reside next to the operator. Action status is used to demonstrate the use case of fence_aws
@razo7
Get the instance ID from the provider ID in AWS
1427bb4 
Remove a prefix from the Provider ID for having the instance ID per machine
@razo7
Fetch the namespace the operator is installed
129ac60 
Instead of hard code the namespace of where the operator is installed just fetch the environment variable OPERATOR_NS so FAR CRs will be created on the same namespace that FAR operator has been installed
razo7 added 3 commits May 2, 2023 18:10
@razo7
Delete redundant function
f78fccd 
PR medik8s#42 add downward API which makes some code redundant
@razo7
Better import name and signal missing env var
b50db0d 
Meaningful and less confusing import names, and signal for GetDeploymentNamespace failure but don't faile e2e/ut test
@razo7
Don't expose cluster secrets
0497740 
Bad practice
@razo7
Copy link
Member Author

razo7 commented May 4, 2023

Yay! All green

clobrano
clobrano reviewed May 4, 2023
View reviewed changes
test/e2e/far_e2e_test.go Outdated Show resolved Hide resolved
mshitrit
mshitrit reviewed May 4, 2023
View reviewed changes
pkg/utils/pods.go Outdated Show resolved Hide resolved
mshitrit
mshitrit reviewed May 4, 2023
View reviewed changes
test/e2e/e2e_suite_test.go Outdated Show resolved Hide resolved
mshitrit
mshitrit reviewed May 4, 2023
View reviewed changes
test/e2e/far_e2e_test.go
@@ -39,12 +69,64 @@ var _ = Describe("FAR E2e", func() {
Expect(k8sClient.Get(context.Background(), client.ObjectKeyFromObject(far), testFarCR)).To(Succeed(), "failed to get FAR CR")
})
})

Context("fence agent - non-Dummy", func() {
Copy link
Member

Choose a reason for hiding this comment

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

maybe a better name than non-Dummy ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was using the term dummy for FA which is dumb and does not act as a real FA in comparison to a real one who
supports some FA actions and actually communicate with a machine/node.

Is fence agent - fence_aws or fence_ipmilan sounds better to you?
Anyway will address this possible change in #20 and merge the current PR.

mshitrit
mshitrit reviewed May 4, 2023
View reviewed changes
test/e2e/far_e2e_test.go Outdated Show resolved Hide resolved
mshitrit
mshitrit reviewed May 4, 2023
View reviewed changes
test/e2e/far_e2e_test.go Outdated Show resolved Hide resolved
@razo7
Typo fix + use log
0d9572b 
Due to code-review
@mshitrit
Copy link
Member

mshitrit commented May 7, 2023

/lgtm
/hold
As far as I'm concerned only nits - feel free to unhold/fix further/wait for more feedback

@openshift-ci openshift-ci bot added the lgtm label May 7, 2023
@razo7
Copy link
Member Author

razo7 commented May 7, 2023

/unhold

@openshift-merge-robot openshift-merge-robot merged commit a7916ab into medik8s:main May 7, 2023
@razo7 razo7 mentioned this pull request May 9, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clobrano clobrano clobrano left review comments

@mshitrit mshitrit mshitrit left review comments

@beekhof beekhof Awaiting requested review from beekhof

Assignees

@mshitrit mshitrit

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@razo7 @mshitrit @clobrano @openshift-merge-robot