Skip to content

Conversation

chiragkyal
Copy link
Member

@chiragkyal chiragkyal commented Aug 12, 2024

Description of the change:
This PR

Steps followed

  • Install Operator SDK Binary v1.36.1
export ARCH=$(case $(uname -m) in x86_64) echo -n amd64 ;; aarch64) echo -n arm64 ;; *) echo -n $(uname -m) ;; esac)
export OS=$(uname | awk '{print tolower($0)}')

export OPERATOR_SDK_DL_URL=https://github.com/operator-framework/operator-sdk/releases/download/v1.36.1
curl -LO ${OPERATOR_SDK_DL_URL}/operator-sdk_${OS}_${ARCH}

chmod +x operator-sdk_${OS}_${ARCH} && sudo mv operator-sdk_${OS}_${ARCH} /usr/local/bin/operator-sdk
  • Generated the testdata using hack/generate/samples/generate_testdata.go script
    • Applied the following patch locally to create the MemcachedSample instead of MoleculeSample to be used in the e2e job.
    • Note: Once the scaffolding is done, revert back to the original generate_testdata.go. We can follow up on creating the testdata without applying this patch.
diff --git a/hack/generate/samples/generate_testdata.go b/hack/generate/samples/generate_testdata.go
index ebf07ec..a6d328b 100644
--- a/hack/generate/samples/generate_testdata.go
+++ b/hack/generate/samples/generate_testdata.go
@@ -49,9 +49,9 @@ func main() {
        }
 
        // samplesPath is the path where all samples should be generated
-       samplesPath := filepath.Join(wd, "testdata")
+       samplesPath := filepath.Join(wd, "openshift", "ci", "testdata")
        log.Infof("writing sample directories under %s", samplesPath)
 
        log.Infof("creating Ansible Memcached Sample")
-       ansible.GenerateMoleculeSample(samplesPath)
+       ansible.GenerateMemcachedSamples(samplesPath)
 }
go run hack/generate/samples/generate_testdata.go

Motivation for the change:
Add existing downstream e2e tests

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 12, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Aug 12, 2024

@chiragkyal: This pull request references CFE-1100 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

Description of the change:

Motivation for the change:

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 openshift-eng/jira-lifecycle-plugin repository.

Copy link

openshift-ci bot commented Aug 12, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 12, 2024
Copy link

openshift-ci bot commented Aug 12, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chiragkyal

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 Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 12, 2024
@chiragkyal
Copy link
Member Author

/test images

@arkadeepsen
Copy link
Member

/test all

@arkadeepsen
Copy link
Member

@chiragkyal Let's add the CI config to get this test running on the main branch PRs.

@arkadeepsen
Copy link
Member

arkadeepsen commented Aug 14, 2024

/hold
Until e2e CI job gives a green signal

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 14, 2024
@chiragkyal chiragkyal marked this pull request as ready for review August 14, 2024 08:29
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 14, 2024
@openshift-ci openshift-ci bot requested review from arkadeepsen and oceanc80 August 14, 2024 08:30
Comment on lines 146 to 158
.PHONY: ansible-operator
ANSIBLE_OPERATOR = $(shell pwd)/bin/ansible-operator
ansible-operator: ## Download ansible-operator locally if necessary, preferring the $(pwd)/bin path over global if both exist.
ifeq (,$(wildcard $(ANSIBLE_OPERATOR)))
ifeq (,$(shell which ansible-operator 2>/dev/null))
@{ \
set -e ;\
mkdir -p $(dir $(ANSIBLE_OPERATOR)) ;\
curl -sSLo $(ANSIBLE_OPERATOR) https://github.com/operator-framework/operator-sdk/releases/download/v1.31.0/ansible-operator_$(OS)_$(ARCH) ;\
chmod +x $(ANSIBLE_OPERATOR) ;\
}
else
ANSIBLE_OPERATOR = $(shell which ansible-operator)
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 want to take a closer look if this can be removed / updated

Copy link
Member

Choose a reason for hiding this comment

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

This is generated when using code scaffolding for creating the testdata directory. This is not used in the e2e script that you added.

Copy link
Member

Choose a reason for hiding this comment

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

@chiragkyal Are you planning to make any changes here?

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've commented out ansible-operator, operator-sdk, opm targets in 4027222

Copy link
Member

Choose a reason for hiding this comment

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

Please also ensure other make targets are not impacted by removal of the above targets.

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've made some clean up in 22b4c24 so that other (necessary) make targets are not impacted.

Copy link
Member Author

Choose a reason for hiding this comment

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

This discussion is now outdated because of #12 (comment). The hack/generate/samples/generate_testdata.go script does it out of the box.

@arkadeepsen
Copy link
Member

/test ?

Copy link

openshift-ci bot commented Aug 14, 2024

@arkadeepsen: The following commands are available to trigger required jobs:

  • /test e2e-ansible
  • /test images
  • /test sanity
  • /test unit

Use /test all to run all jobs.

In response to this:

/test ?

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-sigs/prow repository.

@arkadeepsen
Copy link
Member

/test e2e-ansible

@arkadeepsen
Copy link
Member

/test images

@openshift-ci-robot
Copy link

openshift-ci-robot commented Aug 14, 2024

@chiragkyal: This pull request references CFE-1100 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

Description of the change:
This PR

Motivation for the change:
Add existing downstream e2e tests

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 openshift-eng/jira-lifecycle-plugin repository.

@arkadeepsen
Copy link
Member

/hold Until e2e CI job gives a green signal

/hold cancel
The e2e CI job has given a green signal

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 19, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Aug 21, 2024

@chiragkyal: This pull request references CFE-1100 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

Description of the change:
This PR

Steps followed

  • Install Operator SDK Binary v1.36.1
export ARCH=$(case $(uname -m) in x86_64) echo -n amd64 ;; aarch64) echo -n arm64 ;; *) echo -n $(uname -m) ;; esac)
export OS=$(uname | awk '{print tolower($0)}')

export OPERATOR_SDK_DL_URL=https://github.com/operator-framework/operator-sdk/releases/download/v1.36.1
curl -LO ${OPERATOR_SDK_DL_URL}/operator-sdk_${OS}_${ARCH}

chmod +x operator-sdk_${OS}_${ARCH} && sudo mv operator-sdk_${OS}_${ARCH} /usr/local/bin/operator-sdk
  • Generated the testdata using hack/generate/samples/generate_testdata.go script
    • Applied the following patch locally to create the MemcachedSample instead of MoleculeSample to be used in the e2e job.
    • Note: Once the scaffolding is done, revert back to the original generate_testdata.go. We can follow up on creating the testdata without applying this patch.
diff --git a/hack/generate/samples/generate_testdata.go b/hack/generate/samples/generate_testdata.go
index ebf07ec..a6d328b 100644
--- a/hack/generate/samples/generate_testdata.go
+++ b/hack/generate/samples/generate_testdata.go
@@ -49,9 +49,9 @@ func main() {
       }

       // samplesPath is the path where all samples should be generated
-       samplesPath := filepath.Join(wd, "testdata")
+       samplesPath := filepath.Join(wd, "openshift", "ci", "testdata")
       log.Infof("writing sample directories under %s", samplesPath)

       log.Infof("creating Ansible Memcached Sample")
-       ansible.GenerateMoleculeSample(samplesPath)
+       ansible.GenerateMemcachedSamples(samplesPath)
}
go run hack/generate/samples/generate_testdata.go

Motivation for the change:
Add existing downstream e2e tests

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 openshift-eng/jira-lifecycle-plugin repository.

@chiragkyal
Copy link
Member Author

@chiragkyal There are many references to v1.31.0 in the openshift/ci/testdata directory. You can update them to v1.35.0 as we have rebased to it.

As discussed, I've scaffolded the testdata using hack/generate/samples/generate_testdata.go script. Steps are present in the PR description #12 (comment)

@arkadeepsen
Copy link
Member

/retest

@arkadeepsen
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 22, 2024
@arkadeepsen
Copy link
Member

/cc @KeenonLee
For qe-approved label

@openshift-ci openshift-ci bot requested a review from KeenonLee August 22, 2024 03:11

# Install oc client
if ! [ -x "$(command -v oc)" ]; then
curl -Lo oc.tar.gz https://github.com/openshift/origin/releases/download/v3.11.0/openshift-origin-client-tools-v3.11.0-0cbc58b-linux-64bit.tar.gz
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this refer to latest openshift release?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, updated to "4.16.0"

@arkadeepsen
Copy link
Member

/hold
until #12 (comment) is addressed

@openshift-ci openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed lgtm Indicates that a PR is ready to be merged. labels Aug 22, 2024
@KeenonLee
Copy link
Member

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Aug 22, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Aug 22, 2024

@chiragkyal: This pull request references CFE-1100 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

Description of the change:
This PR

Steps followed

  • Install Operator SDK Binary v1.36.1
export ARCH=$(case $(uname -m) in x86_64) echo -n amd64 ;; aarch64) echo -n arm64 ;; *) echo -n $(uname -m) ;; esac)
export OS=$(uname | awk '{print tolower($0)}')

export OPERATOR_SDK_DL_URL=https://github.com/operator-framework/operator-sdk/releases/download/v1.36.1
curl -LO ${OPERATOR_SDK_DL_URL}/operator-sdk_${OS}_${ARCH}

chmod +x operator-sdk_${OS}_${ARCH} && sudo mv operator-sdk_${OS}_${ARCH} /usr/local/bin/operator-sdk
  • Generated the testdata using hack/generate/samples/generate_testdata.go script
    • Applied the following patch locally to create the MemcachedSample instead of MoleculeSample to be used in the e2e job.
    • Note: Once the scaffolding is done, revert back to the original generate_testdata.go. We can follow up on creating the testdata without applying this patch.
diff --git a/hack/generate/samples/generate_testdata.go b/hack/generate/samples/generate_testdata.go
index ebf07ec..a6d328b 100644
--- a/hack/generate/samples/generate_testdata.go
+++ b/hack/generate/samples/generate_testdata.go
@@ -49,9 +49,9 @@ func main() {
       }

       // samplesPath is the path where all samples should be generated
-       samplesPath := filepath.Join(wd, "testdata")
+       samplesPath := filepath.Join(wd, "openshift", "ci", "testdata")
       log.Infof("writing sample directories under %s", samplesPath)

       log.Infof("creating Ansible Memcached Sample")
-       ansible.GenerateMoleculeSample(samplesPath)
+       ansible.GenerateMemcachedSamples(samplesPath)
}
go run hack/generate/samples/generate_testdata.go

Motivation for the change:
Add existing downstream e2e tests

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 openshift-eng/jira-lifecycle-plugin repository.

# Install oc client
if ! [ -x "$(command -v oc)" ]; then
OPENSHIFT_CLIENT_VERSION="4.16.0"
curl -Lo oc.tar.gz https://mirror.openshift.com/pub/openshift-v4/clients/ocp/$OPENSHIFT_CLIENT_VERSION/openshift-client-linux-$OPENSHIFT_CLIENT_VERSION.tar.gz
Copy link
Member

Choose a reason for hiding this comment

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

Better to use curly braces for the shell parameter expansion to avoid issues and confusion: https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html

Suggested change
curl -Lo oc.tar.gz https://mirror.openshift.com/pub/openshift-v4/clients/ocp/$OPENSHIFT_CLIENT_VERSION/openshift-client-linux-$OPENSHIFT_CLIENT_VERSION.tar.gz
curl -Lo oc.tar.gz https://mirror.openshift.com/pub/openshift-v4/clients/ocp/${OPENSHIFT_CLIENT_VERSION}/openshift-client-linux-${OPENSHIFT_CLIENT_VERSION}.tar.gz

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, updated to use ${parameter}

Signed-off-by: chiragkyal <ckyal@redhat.com>
@arkadeepsen
Copy link
Member

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 22, 2024
Copy link

openshift-ci bot commented Aug 22, 2024

@chiragkyal: all tests passed!

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-sigs/prow repository. I understand the commands that are listed here.

@arkadeepsen
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 22, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit dca970a into openshift:main Aug 22, 2024
@arkadeepsen
Copy link
Member

/cherrypick release-4.17

@openshift-cherrypick-robot
Copy link
Contributor

@arkadeepsen: new pull request created: #17

In response to this:

/cherrypick release-4.17

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-sigs/prow repository.

@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

Distgit: openshift-enterprise-ansible-operator
This PR has been included in build openshift-enterprise-ansible-operator-container-v4.18.0-202408221511.p0.gdca970a.assembly.stream.el8.
All builds following this will include this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. qe-approved Signifies that QE has signed off on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants