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

Container2step #300

Merged
merged 15 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion fre/gfdl_msd_schemas
4 changes: 3 additions & 1 deletion fre/make/create_docker_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,16 @@ def dockerfile_create(yamlfile, platform, target, execute):
## Check for type of build
if platform["container"] is True:
image=modelYaml.platforms.getContainerImage(platformName)
stage2image = modelYaml.platforms.getContainer2base(platformName)
bldDir = platform["modelRoot"] + "/" + fremakeYaml["experiment"] + "/exec"
tmpDir = "tmp/"+platformName
dockerBuild = buildDocker.container(base = image,
exp = fremakeYaml["experiment"],
libs = fremakeYaml["container_addlibs"],
RUNenv = platform["RUNenv"],
target = targetObject,
mkTemplate = platform["mkTemplate"])
mkTemplate = platform["mkTemplate"],
stage2base = stage2image)
dockerBuild.writeDockerfileCheckout("checkout.sh", tmpDir+"/checkout.sh")
dockerBuild.writeDockerfileMakefile(tmpDir+"/Makefile", tmpDir+"/linkline.sh")

Expand Down
28 changes: 18 additions & 10 deletions fre/make/gfdlfremake/buildDocker.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,12 @@ class container():
- RUNenv : The commands that have to be run at
the beginning of a RUN in the dockerfile
to set up the environment
- target : The FRE target
- mkTemplate: The mkmf template to use
- stage2base: The base for the second stage. Empty
string if there is no second stage
"""
def __init__(self,base,exp,libs,RUNenv,target,mkTemplate):
def __init__(self,base,exp,libs,RUNenv,target,mkTemplate,stage2base):
"""
Initialize variables and write to the dockerfile
"""
Expand All @@ -31,6 +35,7 @@ def __init__(self,base,exp,libs,RUNenv,target,mkTemplate):
self.mkmf = True
self.target = target
self.template = mkTemplate
self.stage2base = stage2base

# Set up spack loads in RUN commands in dockerfile
if RUNenv == "":
Expand All @@ -54,15 +59,15 @@ def __init__(self,base,exp,libs,RUNenv,target,mkTemplate):
" && src_dir="+self.src+" \\ \n",
" && mkmf_template="+self.template+ " \\ \n"]
self.d=open("Dockerfile","w")
self.d.writelines("FROM "+self.base+" \n")
if self.base == "ecpe4s/noaa-intel-prototype:2023.09.25":
self.prebuild = '''RUN
'''
self.postbuild = '''
'''
self.secondstage = '''
'''

self.d.writelines("FROM "+self.base+" as builder\n")
## Set up the second stage build list of lines to add
if self.stage2base == "":
self.secondstage = ["\n"]
else:
self.secondstage = [f"FROM {self.stage2base} as final\n",
f"COPY --from=builder {self.src} {self.src}\n",
f"COPY --from=builder {self.bld} {self.bld}\n",
f"ENV PATH=$PATH:{self.bld}\n"]
def writeDockerfileCheckout(self, cScriptName, cOnDisk):
"""
Brief: writes to the checkout part of the Dockerfile and sets up the compile
Expand Down Expand Up @@ -201,6 +206,9 @@ def writeRunscript(self,RUNenv,containerRun,runOnDisk):
self.d.write(" cd "+self.bld+" && make -j 4 "+self.target.getmakeline_add()+"\n")
else:
self.d.write(" && cd "+self.bld+" && make -j 4 "+self.target.getmakeline_add()+"\n")
## Write any second stage lines here
for l in self.secondstage:
self.d.write(l)
self.d.write('ENTRYPOINT ["/bin/bash"]')
self.d.close()

Expand Down
45 changes: 33 additions & 12 deletions fre/make/gfdlfremake/platformfre.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,35 +41,49 @@ def __init__(self,platforminfo):
## Check if we are working with a container and get the info for that
try:
p["container"]
## When not doing a container build, this should all be set to empty strings and Falses
except:
p["container"] = False
p["RUNenv"] = [""]
p["containerBuild"] = ""
p["containerRun"] = ""
p["containerViews"] = False
p["containerBase"] = ""
p["container2step"] = ""
p["container2step"] = False
p["container2base"] = ""
if p["container"]:
## Check the container builder
try:
p["containerBuild"]
except:
raise Exception("You must specify the program used to build the container (containerBuild) on the "+p["name"]+" platform in the file "+fname+"\n")
raise Exception("Platform "+p["name"]+": You must specify the program used to build the container (containerBuild) on the "+p["name"]+" platform in the file "+fname+"\n")
if p["containerBuild"] != "podman" and p["containerBuild"] != "docker":
raise ValueError("Container builds only supported with docker or podman, but you listed "+p["containerBuild"]+"\n")
## Check for container environment set up for RUN commands
raise ValueError("Platform "+p["name"]+": Container builds only supported with docker or podman, but you listed "+p["containerBuild"]+"\n")
## Get the name of the base container
try:
p["containerBase"]
except NameError:
print("You must specify the base container you wish to use to build your application")
try:
p["containerViews"]
except:
p["containerViews"] = False
raise NameError("Platform "+p["name"]+": You must specify the base container you wish to use to build your application")
## Check if this is a 2 step (multi stage) build
try:
p["container2step"]
Copy link
Member

Choose a reason for hiding this comment

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

is this try/except clause just checking to see if a dictionary's attribute is defined at all? or is it about the boolean value assigned elsewhere?

if it's about being defined- how about using p.get("key"), then checking for None? or catching the KeyError explicitly?

Copy link
Collaborator

Choose a reason for hiding this comment

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

A bit of both I think? With some platforms, the container key doesn't even need to be defined. So the try/except checks for if it's even there under a certain platform's information (and not just that it's not defined, being None. If it doesn't exist, it'll set things as empty or False for the next if statement it goes through

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think maybe this could be done a different way that's slightly easier to follow but my brain keeps going back and forth on logic and is confusing itself. Not sure what's the best way

except:
p["container2step"] = ""
p["container2step"] = False
## Get the base for the second stage of the build
if p["container2step"]:
try:
p["container2base"]
except:
raise NameError ("Platform "+p["name"]+": container2step is True, so you must define a container2base\n")
## Check if there is anything special to copy over
else:
## There should not be a second base if this is not a 2 step build
try:
p["container2base"]
except:
p["container2base"] = ""
else:
raise ValueError ("Platform "+p["name"]+": You defined container2base "+p["container2base"]+" but container2step is False\n")
## Get any commands to execute in the dockerfile RUN command
try:
p["RUNenv"]
except:
Expand All @@ -82,6 +96,7 @@ def __init__(self,platforminfo):
if p["containerRun"] != "apptainer" and p["containerRun"] != "singularity":
raise ValueError("Container builds only supported with apptainer, but you listed "+p["containerRun"]+"\n")
else:
## Find the location of the mkmf template
try:
p["mkTemplate"]
except:
Expand Down Expand Up @@ -119,7 +134,6 @@ def getContainerInfoFromName(self,name):
p["RUNenv"], \
p["containerBuild"], \
p["containerRun"], \
p["containerViews"], \
p["containerBase"], \
p["container2step"])
def isContainer(self, name):
Expand All @@ -136,3 +150,10 @@ def getContainerImage(self,name):
for p in self.yaml:
if p["name"] == name:
return p["containerBase"]
def getContainer2base(self,name):
"""
Brief: returns the image to be used in the second step of the Dockerfile
"""
for p in self.yaml:
if p["name"] == name:
return p["container2base"]
4 changes: 3 additions & 1 deletion fre/make/run_fremake_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ def fremake_run(yamlfile, platform, target, parallel, jobs, no_parallel_checkout
###################### container stuff below #######################################
## Run the checkout script
image=modelYaml.platforms.getContainerImage(platformName)
stage2image = modelYaml.platforms.getContainer2base(platformName)
srcDir = platform["modelRoot"] + "/" + fremakeYaml["experiment"] + "/src"
bldDir = platform["modelRoot"] + "/" + fremakeYaml["experiment"] + "/exec"
tmpDir = "tmp/"+platformName
Expand Down Expand Up @@ -174,7 +175,8 @@ def fremake_run(yamlfile, platform, target, parallel, jobs, no_parallel_checkout
libs = fremakeYaml["container_addlibs"],
RUNenv = platform["RUNenv"],
target = target,
mkTemplate = platform["mkTemplate"])
mkTemplate = platform["mkTemplate"],
stage2base = stage2image)
dockerBuild.writeDockerfileCheckout("checkout.sh", tmpDir+"/checkout.sh")
dockerBuild.writeDockerfileMakefile(freMakefile.getTmpDir() + "/Makefile",
freMakefile.getTmpDir() + "/linkline.sh")
Expand Down
11 changes: 11 additions & 0 deletions fre/make/tests/null_example/platforms.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,14 @@ platforms:
compiler: gnu
mkTemplate: /__w/fre-cli/fre-cli/mkmf/templates/linux-ubuntu-xenial-gnu.mk
modelRoot: ${TEST_BUILD_DIR}/fremake_canopy/test
- name: con.twostep
compiler: intel
RUNenv: [". /spack/share/spack/setup-env.sh", "spack load libyaml", "spack load [email protected]", "spack load [email protected]"]
modelRoot: /apps
container: True
containerBuild: "podman"
containerRun: "apptainer"
containerBase: "ecpe4s/noaa-intel-prototype:2023.09.25"
mkTemplate: "/apps/mkmf/templates/hpcme-intel21.mk"
container2step: True
container2base: "ecpe4s/noaa-intel-prototype:2023.09.25"
11 changes: 11 additions & 0 deletions fre/make/tests/test_create_makefile.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
YAMLFILE = "null_model.yaml"
BM_PLATFORM = ["ncrc5.intel23"]
CONTAINER_PLATFORM = ["hpcme.2023"]
CONTAINER_PLAT2 = ["con.twostep"]
TARGET = ["debug"]
EXPERIMENT = "null_model_full"

Expand Down Expand Up @@ -73,3 +74,13 @@ def test_container_makefile_creation():
create_makefile_script.makefile_create(yamlfile_path,CONTAINER_PLATFORM,TARGET)

assert Path(f"tmp/{container_plat}/Makefile").exists()

def test_container2step_makefile_creation():
"""
Check the makefile is created when the two stage container is used
"""
container_plat = CONTAINER_PLAT2[0]
yamlfile_path = f"{TEST_DIR}/{NM_EXAMPLE}/{YAMLFILE}"
create_makefile_script.makefile_create(yamlfile_path,CONTAINER_PLAT2,TARGET)

assert Path(f"tmp/{container_plat}/Makefile").exists()
26 changes: 26 additions & 0 deletions fre/make/tests/test_run_fremake.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
YAMLPATH = f"{YAMLDIR}/{YAMLFILE}"
PLATFORM = [ "ci.gnu" ]
CONTAINER_PLATFORM = ["hpcme.2023"]
CONTAINER_PLAT2 = ["con.twostep"]
TARGET = ["debug"]
BADOPT = ["foo"]
EXPERIMENT = "null_model_full"
Expand Down Expand Up @@ -125,6 +126,31 @@ def test_run_fremake_run_script_creation_container():
''' checks (internal) container run script creation from previous test '''
assert Path(f"tmp/{CONTAINER_PLATFORM[0]}/execrunscript.sh").exists()

# tests container 2 stage build script/makefile/dockerfile creation
def test_run_fremake_container():
'''run run-fremake with options for containerized build'''
run_fremake_script.fremake_run(YAMLPATH, CONTAINER_PLAT2, TARGET, False, 1, True, False, VERBOSE)
Copy link
Member

Choose a reason for hiding this comment

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

are there any return-codes to check?

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 is a copy/paste of the single stage tests.


def test_run_fremake_build_script_creation_container():
''' checks container build script creation from previous test '''
assert Path("createContainer.sh").exists()

def test_run_fremake_dockerfile_creation_container():
''' checks dockerfile creation from previous test '''
assert Path("Dockerfile").exists()

def test_run_fremake_checkout_script_creation_container():
''' checks checkout script creation from previous test '''
assert Path(f"tmp/{CONTAINER_PLAT2[0]}/checkout.sh").exists()

def test_run_fremake_makefile_creation_container():
''' checks makefile creation from previous test '''
assert Path(f"tmp/{CONTAINER_PLAT2[0]}/Makefile").exists()

def test_run_fremake_run_script_creation_container():
''' checks (internal) container run script creation from previous test '''
assert Path(f"tmp/{CONTAINER_PLAT2[0]}/execrunscript.sh").exists()

Comment on lines +134 to +153
Copy link
Member

@ilaflott ilaflott Jan 21, 2025

Choose a reason for hiding this comment

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

these shouldn't be separate- lets do something like

def test_run_fremake_container():
    run_fremake_script.fremake_run(
        YAMLPATH, CONTAINER_PLAT2, TARGET, False, 1, True, False, VERBOSE)
    assert all( [ condition1,
                  condition2,
                  ...
                  conditionN ] )

Copy link
Member Author

@thomas-robinson thomas-robinson Jan 28, 2025

Choose a reason for hiding this comment

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

What's done here is consistent with the rest of the testing done in this file.

# tests for builds with multiple targets

def test_run_fremake_bad_target():
Expand Down
6 changes: 0 additions & 6 deletions fre/yamltools/tests/AM5_example/compile_yamls/platforms.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,18 @@ platforms:
compiler: intel
modulesInit: [" module use -a /ncrc/home2/fms/local/modulefiles \n","source $MODULESHOME/init/sh \n"]
modules: [ !join [*INTEL, "/2022.2.1"],"fre/bronx-20",cray-hdf5/1.12.2.3, cray-netcdf/4.9.0.3]
fc: ftn
cc: cc
mkTemplate: "/ncrc/home2/fms/local/opt/fre-commands/bronx-20/site/ncrc5/$(INTEL).mk"
modelRoot: ${HOME}/fremake_canopy/test
- name: ncrc5.intel23
compiler: intel
modulesInit: [" module use -a /ncrc/home2/fms/local/modulefiles \n","source $MODULESHOME/init/sh \n"]
modules: [!join [*INTEL, "/2023.1.0"],"fre/bronx-20",cray-hdf5/1.12.2.3, cray-netcdf/4.9.0.3]
fc: ftn
cc: cc
mkTemplate: "/ncrc/home2/fms/local/opt/fre-commands/bronx-20/site/ncrc5/$(INTEL).mk"
modelRoot: ${HOME}/fremake_canopy/test
- name: hpcme.2023
compiler: intel
RUNenv: [". /spack/share/spack/setup-env.sh", "spack load libyaml", "spack load [email protected]", "spack load [email protected]"]
modelRoot: /apps
fc: mpiifort
cc: mpiicc
container: True
containerBuild: "podman"
containerRun: "apptainer"