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

Just testing the new ksctl register member command #1032

Closed

Conversation

mfrancisc
Copy link
Contributor

Paired with: kubesaw/ksctl#53

Will close once verified.

Copy link

openshift-ci bot commented Aug 13, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mfrancisc

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

@mfrancisc mfrancisc changed the title Just testing new new ksctl register member command Just testing the new ksctl register member command Aug 13, 2024
@MatousJobanek
Copy link
Collaborator

@mfrancisc it's related - se my comment in the other PR: kubesaw/ksctl#53 (comment)

@mfrancisc
Copy link
Contributor Author

@mfrancisc it's related - se my comment in the other PR: kubesaw/ksctl#53 (comment)

Thanks I was looking into it locally, I've inverted the SAs apparently 🤦‍♂️

@mfrancisc
Copy link
Contributor Author

/retest

updated the ksctl PR

@MatousJobanek
Copy link
Collaborator

yup, the pairing was broken - I didn't realize that the if statement is evaluated at the beginning of the makefile target. Here is the fix #1034

@MatousJobanek
Copy link
Collaborator

/retest
infra

@MatousJobanek
Copy link
Collaborator

DEPLOY_LATEST is set to true - no pairing

🤦

@mfrancisc
Copy link
Contributor Author

I see later it installs from my branch , but it doesn't run the register-member command anymore:

``` detected branch registermember # check if a branch with the same ref exists in the user's fork of ksctl repo branches of https://github.com/mfrancisc/ksctl - checking if there is a branch registermember we could pair with. curl https://github.com/mfrancisc/ksctl.git/info/refs?service=git-upload-pack --output - % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed

0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
100 513 0 513 0 0 13500 0 --:--:-- --:--:-- --:--:-- 13500
001e# service=git-upload-pack
00000155a0a148a9988b4a2287dc85b744f4439c35c8e559 HEAD�multi_ack thin-pack side-band side-band-64k ofs-delta shallow deepen-since deepen-not deepen-relative no-progress include-tag multi_ack_detailed allow-tip-sha1-in-want allow-reachable-sha1-in-want no-done symref=HEAD:refs/heads/master filter object-format=sha1 agent=git/github-7d8f01c5c0e4
003fa0a148a9988b4a2287dc85b744f4439c35c8e559 refs/heads/master
0047635fb8cccba3b1466cdae26d3c1c4e2b91481e27 refs/heads/registermember
0000# check if the branch with the same name exists, if so then merge it with master and use the merge branch, if not then use master
which: no yamllint in (/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/bin:/usr/local/go/bin:/cli)
Branch ref of the user's fork to be used for pairing: "refs/heads/registermember"

define temp dir

delete to have clear environment

rm -rf /tmp/ksctl
git config --global user.email "[email protected]"
git config --global user.name "Devtools"

clone

git clone --depth=1 https://github.com/kubesaw/ksctl.git /tmp/ksctl
Cloning into '/tmp/ksctl'...

add the user's fork as remote repo

git --git-dir=/tmp/ksctl/.git --work-tree=/tmp/ksctl remote add external https://github.com/mfrancisc/ksctl.git

fetch the branch

git --git-dir=/tmp/ksctl/.git --work-tree=/tmp/ksctl fetch external refs/heads/registermember
From https://github.com/mfrancisc/ksctl

  • branch registermember -> FETCH_HEAD
  • [new branch] registermember -> external/registermember

merge the branch with master

git --git-dir=/tmp/ksctl/.git --work-tree=/tmp/ksctl merge --allow-unrelated-histories --no-commit FETCH_HEAD
Updating 0ffc21a..635fb8c
Fast-forward
pkg/cmd/adm/register_member.go | 390 +++++++++++++++++----------
pkg/cmd/adm/register_member_test.go | 508 ++++++++++++-----------------------
pkg/cmd/generate/cli_configs.go | 6 +-
pkg/cmd/generate/cli_configs_test.go | 2 +-
pkg/utils/util.go | 16 +-
5 files changed, 443 insertions(+), 479 deletions(-)
which: no yamllint in (/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/bin:/usr/local/go/bin:/cli)
Installing ksctl from directory /tmp/ksctl
make -C /tmp/ksctl GOBIN=/go/src/github.com/codeready-toolchain/toolchain-e2e/build/_output/bin install
CGO_ENABLED=0
go install
-ldflags "-X github.com/kubesaw/ksctl/pkg/version.Commit=635fb8cccba3b1466cdae26d3c1c4e2b91481e27 -X github.com/kubesaw/ksctl/pkg/version.BuildTime=date -u '+%Y-%m-%dT%H:%M:%SZ'"
github.com/kubesaw/ksctl/cmd/ksctl/...
rm -rf /tmp/e2e-tiers-out 2>/dev/null || true
/go/src/github.com/codeready-toolchain/toolchain-e2e/build/_output/bin/ksctl generate nstemplatetiers --source deploy/nstemplatetiers --out-dir /tmp/e2e-tiers-out

</details>

@MatousJobanek
Copy link
Collaborator

The thing is that the migration suite installs the latest operators first and then it registers the member clusters. I didn't realize that the registering member clusters still happens in the context of the latest install and that the env var is still set for that command.
let me fix it one more time and correctly

@MatousJobanek
Copy link
Collaborator

the pairing that you see is for a later execution when ksctl is used to generate the appstudio NSTemplateTier files

Copy link

sonarcloud bot commented Aug 26, 2024

@mfrancisc
Copy link
Contributor Author

closing as not needed anymore, the new register-member command was merged.

@mfrancisc mfrancisc closed this Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants