[SPARK-40384][INFRA] Only do base image real in time build when infra dockerfile is changed

[Yikun]

What changes were proposed in this pull request?

Do base image real in time build only when infra dockerfile is changed.

Why are the changes needed?

Recently github ghcr is unstable, so we better use infra static image when no infra dockerfile changes.

After this change even github ghcr image write has some issue, only infra dockerfile PR is blocked, other PR don't have any impact.

Does this PR introduce any user-facing change?

No

How was this patch tested?

1. CI passed, and use the ghcr.io/apache/spark/apache-spark-github-action-image-cache:master-static.
2. local test infra changes trigger CI: Test infra change Yikun/spark#170
3. test ./dev/is-changed.py -m infra-image
   

# previous hash of dockerfile is changed
$ export APACHE_SPARK_REF=871152bda69a5d5db714eecd42fdd3e7d2e04557
$ ./dev/is-changed.py -m infra-image
true
# hash of dockerfile is changed
$ export APACHE_SPARK_REF=0830575100c1bcbc89d98a6859fe3b8d46ca2e6e 
$ ./dev/is-changed.py -m infra-image
false
# next hash  of dockerfile is changed
$ export APACHE_SPARK_REF=e6c58c1bd6f64ebfb337348fa6132c0b230dc932
$ ./dev/is-changed.py -m infra-image
false

[Yikun]

cc @HyukjinKwon

[Yikun]

@HyukjinKwon Thanks, I will revisit today, and mark it ready for review.

[Yikun]

Ready to go

[HyukjinKwon]

hmmm .. actually quick question. would that still build the base image when there are other changes together with Dockerfile? Do we return true when root module is detected?

[Yikun]

would that still build the base image when there are other changes together with Dockerfile?

Yes, module_names contains all changed module's name, test_modules will be ['infra-image'], this will return true if infra-image module is detected:

    elif len(set(test_modules).intersection(module_names)) == 0:
        print("false")
        if opts.fail:
            sys.exit(1)
    else:
        print("true")

BTW, If the fork repository is not synchronized to the latest, if the dockerfile changes are included in the synchronized code, it will also return true. You could see test Yikun#170 , infra-image return true, then build and use the base image in lint, sparkr and pyspark. In this case, if GHCR is unstable, only a rebase needed in fork repo.

Do we return true when root module is detected?

No, we only return true when infra dockerfile changes.

If only root module is detected, will first skip this root check

    # `./dev/is-changed.py -m infra-image` == True only when changing the infra dockerfile
    elif ("root" in test_modules or modules.root in changed_modules) and (
        ["infra-image"] != test_modules
    ):

and then do continue check as above (elif and else) return result according infra-image module is detected. The current PR is an real example to show this case, root dectected (because we change the yaml), but didn't change the dockerfile, infra-image return false, then use the master-static image.

[Yikun]

We could merge this in the morning of some day (such as tmr), I'll keep watching.

[HyukjinKwon]

Merged to master.

[Yikun]

• (OK) Test on dockerfile with root
• (OK) Test on root only
• (OK) Test on normal test
• (OK) Test on apache/spark merge commits: base skip and use static-master directly in sparkr, lint and pyspark
• (Need followup)Test on single dockerfile changes: base trigger and only lint job triggered based on base image, need to enable pyspark/sparkr in followup [SPARK-40384][INFRA] Also trigger PySpark and SparkR job when changing dockerfile #37865.
• Test of schedule job: TBD
• Test on branch job (3.2 or 3.3): TBD

[HyukjinKwon]

Offline discussed with @Yikun. Let's revert it out for now (since this isn't a problem if ghcr isn't broken, and to make our CI implementation simple)

[Yikun]

@HyukjinKwon Thanks!