-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Keystone in CRIB #14326
Keystone in CRIB #14326
Conversation
eaad372
to
7abd46d
Compare
6537149
to
f0b0226
Compare
3a10b2a
to
7dfbe68
Compare
AER Report: CI Core ran successfully ✅AER Report: Operator UI CI ran successfully ✅ |
357886c
to
9864099
Compare
7f49528
to
b658c21
Compare
PanicErr(err) | ||
|
||
jsonRPCClient, err := rpc.Dial(ethURL) | ||
insecureSkipVerify := os.Getenv("INSECURE_SKIP_VERIFY") == "true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needed to add this since CRIB can run locally via kind, which uses https but a locally generated cert
src.NewDeployJobSpecsCommand(), | ||
src.NewGenerateCribClusterOverridesCommand(), | ||
src.NewDeleteJobsCommand(), | ||
src.NewProvisionKeystoneCommand(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reviewers: This is the main entrypoint, CRIB scripts call into this command only to provision everything. All other commands below are essentially subcmds of this one
src.NewPreprovisionCribCommand(), | ||
src.NewPostprovisionCribCommand(), | ||
src.NewProvisionCapabilitesRegistryCommand(), | ||
src.NewProvisionStreamsTriggerCommand(), | ||
src.NewDeployAndInitializeCapabilitiesRegistryCommand(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cedric-cordenier Can we remove this? I kept it around because I wasnt sure if the team needs this. NewProvisionCapabilitesRegistryCommand
handles provisioning CR for crib
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we can remove this if NewProvisionCapabilitesRegistryCommand
handles it.
return "provision-keystone" | ||
} | ||
|
||
func (g *provisionKeystone) Run(args []string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reviewers: This is the main entrypoint to keystone in CRIB
Quality Gate failedFailed conditions See analysis details on SonarQube Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
@@ -1,6 +1,6 @@ | |||
module github.com/smartcontractkit/chainlink/tools/goreleaser-config | |||
|
|||
go 1.23.0 | |||
go 1.22.8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i want to say yes, but i do not remember the reasoning behind it now :/. I could try setting it back and seeing it it breaks any tooling
} | ||
|
||
func ensureArtefactsDir(artefactsDir string) { | ||
_, err := os.Stat(artefactsDir) | ||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be checking for a certain kind of error? https://pkg.go.dev/os#IsNotExist
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestMustReadNodesList(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func TestMustReadNodesList(t *testing.T) { | |
func Test_mustReadNodesList(t *testing.T) { |
… into CRIB-322-keystone-evm-contract-deployment-tooling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me
Main request: A README about how to use the tools so that I can push it forward next year
- minimal cmds to start, run, stop
- cfg or permission required to use it
- known limitations
and anything keystone specific that isn't covered in normal crib implementations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me
Main request: A README about how to use the tools so that I can push it forward next year
minimal cmds to start, run, stop
cfg or permission required to use it
known limitations
and anything keystone specific that isn't covered in normal crib implementations
I'll pair with Henry early next week to take notes / record a video on this
PR LGTM at a glance, approving to get out of rebase hell, with further usage next week.
Notes from call: in the future, will need to chainlink/deployments integrated to this. |
Quality Gate passedIssues Measures |
This PR enables keystone in CRIB. In summary, it:
Note that a good portion of this code will be replaced with work done at
chainlink/deployments
. It's better IMO to get this merged in sooner rather than later to begin integration work withchainlink/deployments
ASAP to avoid continuing duplicate implementations of the same provisioning logic.TODO:
make gomodslocalupdate